[PATCH] security: do not log password
It's insecure to log password, nomatter the password is encrypted or
not. And do not log it even in debug mode, in the consideration of
resilience, surposing that the log mode has been modified by the
attacker.
Signed-off-by: Zhang Bo
---
src/libvirt-domain.c| 3 +--
src/qemu/qemu_monitor.c | 3 +--
2 files changed, 2 insertions(+), 4 deletions(-)
diff --git a/src/libvirt-domain.c b/src/libvirt-domain.c
index a12809c2d5..e2a57c178b 100644
--- a/src/libvirt-domain.c
+++ b/src/libvirt-domain.c
@@ -11340,8 +11340,7 @@ virDomainSetUserPassword(virDomainPtr dom,
const char *password,
unsigned int flags)
{
-VIR_DOMAIN_DEBUG(dom, "user=%s, password=%s, flags=0x%x",
- NULLSTR(user), NULLSTR(password), flags);
+VIR_DOMAIN_DEBUG(dom, "user=%s, flags=0x%x", NULLSTR(user), flags);
virResetLastError();
diff --git a/src/qemu/qemu_monitor.c b/src/qemu/qemu_monitor.c
index 9c853ccb93..9bfaf53b65 100644
--- a/src/qemu/qemu_monitor.c
+++ b/src/qemu/qemu_monitor.c
@@ -2241,8 +2241,7 @@ qemuMonitorSetPassword(qemuMonitorPtr mon,
if (!protocol)
return -1;
-VIR_DEBUG("protocol=%s, password=%p, action_if_connected=%s",
- protocol, password, action_if_connected);
+VIR_DEBUG("protocol=%s, action_if_connected=%s", protocol,
action_if_connected);
QEMU_CHECK_MONITOR(mon);
--
2.23.0.windows.1
[PATCHv2 0/5] update tls files without restarting libvirtd
v1: https://www.redhat.com/archives/libvir-list/2020-February/msg00370.html v2: according to Dienial's suggestion: * update each tls file one time -> update all of them at one time * forced to re-create the credentials object, rather than allowing to append to the original ones. Zhang Bo (5): virnetserver: Introduce virNetServerUpdateTlsFiles tls: Add a mutex lock on 'tlsCtxt' admin: Introduce virAdmServerUpdateTlsFiles virt-admin: Introduce command srv-update-tls docs: update virt-admin.rst for server-update-tls docs/manpages/virt-admin.rst | 16 +++ include/libvirt/libvirt-admin.h | 3 ++ src/admin/admin_protocol.x | 12 +- src/admin/admin_server.c | 9 src/admin/admin_server.h | 3 ++ src/admin/libvirt-admin.c| 30 + src/admin/libvirt_admin_private.syms | 1 + src/admin/libvirt_admin_public.syms | 1 + src/libvirt_remote.syms | 1 + src/rpc/virnetserver.c | 51 ++ src/rpc/virnetserver.h | 2 + src/rpc/virnetserverclient.c | 4 ++ src/rpc/virnettlscontext.c | 46 src/rpc/virnettlscontext.h | 3 ++ tools/virt-admin.c | 64 15 files changed, 245 insertions(+), 1 deletion(-) -- 2.23.0.windows.1
[PATCHv2 1/5] virnetserver: Introduce virNetServerUpdateTlsFiles
Add an API to update server's tls context.
---
src/libvirt_remote.syms| 1 +
src/rpc/virnetserver.c | 51 ++
src/rpc/virnetserver.h | 2 ++
src/rpc/virnettlscontext.c | 46 ++
src/rpc/virnettlscontext.h | 3 +++
5 files changed, 103 insertions(+)
diff --git a/src/libvirt_remote.syms b/src/libvirt_remote.syms
index 0493467f46..0018a0c41d 100644
--- a/src/libvirt_remote.syms
+++ b/src/libvirt_remote.syms
@@ -137,6 +137,7 @@ virNetServerSetClientLimits;
virNetServerSetThreadPoolParameters;
virNetServerSetTLSContext;
virNetServerUpdateServices;
+virNetServerUpdateTlsFiles;
# rpc/virnetserverclient.h
diff --git a/src/rpc/virnetserver.c b/src/rpc/virnetserver.c
index 072ffdf5a3..0bfe94d3f8 100644
--- a/src/rpc/virnetserver.c
+++ b/src/rpc/virnetserver.c
@@ -21,6 +21,9 @@
#include
+#include
+#include
+
#include "virnetserver.h"
#include "virlog.h"
#include "viralloc.h"
@@ -1205,3 +1208,51 @@ virNetServerSetClientLimits(virNetServerPtr srv,
virObjectUnlock(srv);
return ret;
}
+
+static virNetTLSContextPtr
+virNetServerGetTLSContext(virNetServerPtr srv)
+{
+size_t i;
+virNetTLSContextPtr ctxt = NULL;
+virNetServerServicePtr svc = NULL;
+
+/* find svcTLS from srv, get svcTLS->tls */
+for (i = 0; i < srv->nservices; i++) {
+svc = srv->services[i];
+ctxt = virNetServerServiceGetTLSContext(svc);
+if (ctxt != NULL)
+break;
+}
+
+return ctxt;
+}
+
+int
+virNetServerUpdateTlsFiles(virNetServerPtr srv)
+{
+int ret = -1;
+virNetTLSContextPtr ctxt = NULL;
+bool privileged = geteuid() == 0 ? true : false;
+
+ctxt = virNetServerGetTLSContext(srv);
+if (!ctxt) {
+VIR_ERROR(_("no tls svc found, unable to update tls files"));
+return -1;
+}
+
+virObjectLock(srv);
+virObjectLock(ctxt);
+
+if (virNetTLSContextReloadForServer(ctxt, !privileged)) {
+VIR_ERROR(_("failed to reload server's tls context"));
+goto cleanup;
+}
+
+VIR_INFO("update tls files success");
+ret = 0;
+
+ cleanup:
+virObjectUnlock(ctxt);
+virObjectUnlock(srv);
+return ret;
+}
diff --git a/src/rpc/virnetserver.h b/src/rpc/virnetserver.h
index 260c99b22d..1c6a2efb6c 100644
--- a/src/rpc/virnetserver.h
+++ b/src/rpc/virnetserver.h
@@ -133,3 +133,5 @@ size_t virNetServerGetCurrentUnauthClients(virNetServerPtr
srv);
int virNetServerSetClientLimits(virNetServerPtr srv,
long long int maxClients,
long long int maxClientsUnauth);
+
+int virNetServerUpdateTlsFiles(virNetServerPtr srv);
diff --git a/src/rpc/virnettlscontext.c b/src/rpc/virnettlscontext.c
index 44f0dfce77..02c17124a1 100644
--- a/src/rpc/virnettlscontext.c
+++ b/src/rpc/virnettlscontext.c
@@ -919,6 +919,52 @@ virNetTLSContextPtr virNetTLSContextNewServer(const char
*cacert,
}
+int virNetTLSContextReloadForServer(virNetTLSContextPtr ctxt,
+bool tryUserPkiPath)
+{
+gnutls_certificate_credentials_t x509credBak;
+int err;
+char *cacert = NULL;
+char *cacrl = NULL;
+char *cert = NULL;
+char *key = NULL;
+
+x509credBak = ctxt->x509cred;
+ctxt->x509cred = NULL;
+
+if (virNetTLSContextLocateCredentials(NULL, tryUserPkiPath, true,
+ , , , ))
+goto error;
+
+err = gnutls_certificate_allocate_credentials(>x509cred);
+if (err) {
+virReportError(VIR_ERR_SYSTEM_ERROR,
+ _("Unable to allocate x509 credentials: %s"),
+ gnutls_strerror(err));
+goto error;
+}
+
+if (virNetTLSContextSanityCheckCredentials(true, cacert, cert))
+goto error;
+
+if (virNetTLSContextLoadCredentials(ctxt, true, cacert, cacrl, cert, key))
+goto error;
+
+gnutls_certificate_set_dh_params(ctxt->x509cred,
+ ctxt->dhParams);
+
+gnutls_certificate_free_credentials(x509credBak);
+
+return 0;
+
+ error:
+if (ctxt->x509cred)
+gnutls_certificate_free_credentials(ctxt->x509cred);
+ctxt->x509cred = x509credBak;
+return -1;
+}
+
+
virNetTLSContextPtr virNetTLSContextNewClient(const char *cacert,
const char *cacrl,
const char *cert,
diff --git a/src/rpc/virnettlscontext.h b/src/rpc/virnettlscontext.h
index f3273bc26a..fe885aed9a 100644
--- a/src/rpc/virnettlscontext.h
+++ b/src/rpc/virnettlscontext.h
@@ -62,6 +62,9 @@ virNetTLSContextPtr virNetTLSContextNewClient(const char
*cacert,
bool sanityCheckCert,
bool requireValidCert);
+int virNetTLSContextReloadForServer(virNetTLSContextPtr ctxt,
+bool tryUserPkiPath);
+
int
[PATCHv2 4/5] virt-admin: Introduce command srv-update-tls
wire-up virAdmServerUpdateTlsFiles API into virt-admin client.
---
tools/virt-admin.c | 64 ++
1 file changed, 64 insertions(+)
diff --git a/tools/virt-admin.c b/tools/virt-admin.c
index 32edfe5757..a8e5e0a5af 100644
--- a/tools/virt-admin.c
+++ b/tools/virt-admin.c
@@ -957,6 +957,60 @@ cmdSrvClientsSet(vshControl *ctl, const vshCmd *cmd)
goto cleanup;
}
+/*
+ * Command srv-update-tls
+ *
+ */
+static const vshCmdInfo info_srv_update_tls_file[] = {
+{.name = "help",
+ .data = N_("notify server to update TLS related files online.")
+},
+{.name = "desc",
+ .data = N_("notify server to update the CA cert, "
+"CA CRL, server cert / key without restarts. "
+"See OPTIONS for currently supported attributes.")
+},
+{.name = NULL}
+};
+
+static const vshCmdOptDef opts_srv_update_tls_file[] = {
+{.name = "server",
+ .type = VSH_OT_DATA,
+ .flags = VSH_OFLAG_REQ,
+ .help = N_("Available servers on a daemon. "
+"Currently only supports 'libvirtd'.")
+},
+{.name = NULL}
+};
+
+static bool
+cmdSrvUpdateTlsFiles(vshControl *ctl, const vshCmd *cmd)
+{
+bool ret = false;
+const char *srvname = NULL;
+
+virAdmServerPtr srv = NULL;
+vshAdmControlPtr priv = ctl->privData;
+
+if (vshCommandOptStringReq(ctl, cmd, "server", ) < 0)
+return false;
+
+if (!(srv = virAdmConnectLookupServer(priv->conn, srvname, 0)))
+goto cleanup;
+
+if (virAdmServerUpdateTlsFiles(srv, 0) < 0) {
+vshError(ctl, "%s", _("Unable to update server's tls related files."));
+goto cleanup;
+}
+
+ret = true;
+vshPrint(ctl, "update tls related files succeed\n");
+
+ cleanup:
+virAdmServerFree(srv);
+return ret;
+}
+
/* --
* Command daemon-log-filters
* --
@@ -1436,6 +1490,16 @@ static const vshCmdDef managementCmds[] = {
.info = info_srv_clients_set,
.flags = 0
},
+{.name = "srv-update-tls",
+ .flags = VSH_CMD_FLAG_ALIAS,
+ .alias = "server-update-tls"
+},
+{.name = "server-update-tls",
+ .handler = cmdSrvUpdateTlsFiles,
+ .opts = opts_srv_update_tls_file,
+ .info = info_srv_update_tls_file,
+ .flags = 0
+},
{.name = "daemon-log-filters",
.handler = cmdDaemonLogFilters,
.opts = opts_daemon_log_filters,
--
2.23.0.windows.1
[PATCHv2 2/5] tls: Add a mutex lock on 'tlsCtxt'
Prevent the handshake function from reading 'tlsCtxt' while
updating 'tlsCtxt'.
---
src/rpc/virnetserverclient.c | 4
1 file changed, 4 insertions(+)
diff --git a/src/rpc/virnetserverclient.c b/src/rpc/virnetserverclient.c
index 4d85ee25d7..657108239f 100644
--- a/src/rpc/virnetserverclient.c
+++ b/src/rpc/virnetserverclient.c
@@ -1114,7 +1114,9 @@ int virNetServerClientInit(virNetServerClientPtr client)
client->tls);
/* Begin the TLS handshake. */
+virObjectLock(client->tlsCtxt);
ret = virNetTLSSessionHandshake(client->tls);
+virObjectUnlock(client->tlsCtxt);
if (ret == 0) {
/* Unlikely, but ... Next step is to check the certificate. */
if (virNetServerClientCheckAccess(client) < 0)
@@ -1435,7 +1437,9 @@ virNetServerClientDispatchHandshake(virNetServerClientPtr
client)
{
int ret;
/* Continue the handshake. */
+virObjectLock(client->tlsCtxt);
ret = virNetTLSSessionHandshake(client->tls);
+virObjectUnlock(client->tlsCtxt);
if (ret == 0) {
/* Finished. Next step is to check the certificate. */
if (virNetServerClientCheckAccess(client) < 0)
--
2.23.0.windows.1
[PATCHv2 3/5] admin: Introduce virAdmServerUpdateTlsFiles
The server needs to use CA certificate, CRL, server certificate/key to
complete the TLS handshake. If these files change, we needed to restart
libvirtd for them to take effect. This API can update the TLS context
*ONLINE* without restarting libvirtd.
---
include/libvirt/libvirt-admin.h | 3 +++
src/admin/admin_protocol.x | 12 ++-
src/admin/admin_server.c | 9 +
src/admin/admin_server.h | 3 +++
src/admin/libvirt-admin.c| 30
src/admin/libvirt_admin_private.syms | 1 +
src/admin/libvirt_admin_public.syms | 1 +
7 files changed, 58 insertions(+), 1 deletion(-)
diff --git a/include/libvirt/libvirt-admin.h b/include/libvirt/libvirt-admin.h
index abf2792926..e414f776e4 100644
--- a/include/libvirt/libvirt-admin.h
+++ b/include/libvirt/libvirt-admin.h
@@ -402,6 +402,9 @@ int virAdmServerSetClientLimits(virAdmServerPtr srv,
int nparams,
unsigned int flags);
+int virAdmServerUpdateTlsFiles(virAdmServerPtr srv,
+ unsigned int flags);
+
int virAdmConnectGetLoggingOutputs(virAdmConnectPtr conn,
char **outputs,
unsigned int flags);
diff --git a/src/admin/admin_protocol.x b/src/admin/admin_protocol.x
index 42e215d23a..7dc6724032 100644
--- a/src/admin/admin_protocol.x
+++ b/src/admin/admin_protocol.x
@@ -181,6 +181,11 @@ struct admin_server_set_client_limits_args {
unsigned int flags;
};
+struct admin_server_update_tls_files_args {
+admin_nonnull_server srv;
+unsigned int flags;
+};
+
struct admin_connect_get_logging_outputs_args {
unsigned int flags;
};
@@ -314,5 +319,10 @@ enum admin_procedure {
/**
* @generate: both
*/
-ADMIN_PROC_CONNECT_SET_LOGGING_FILTERS = 17
+ADMIN_PROC_CONNECT_SET_LOGGING_FILTERS = 17,
+
+/**
+ * @generate: both
+ */
+ADMIN_PROC_SERVER_UPDATE_TLS_FILES = 18
};
diff --git a/src/admin/admin_server.c b/src/admin/admin_server.c
index ba87f701c3..ebc0cfb045 100644
--- a/src/admin/admin_server.c
+++ b/src/admin/admin_server.c
@@ -367,3 +367,12 @@ adminServerSetClientLimits(virNetServerPtr srv,
return 0;
}
+
+int
+adminServerUpdateTlsFiles(virNetServerPtr srv,
+ unsigned int flags)
+{
+virCheckFlags(0, -1);
+
+return virNetServerUpdateTlsFiles(srv);
+}
diff --git a/src/admin/admin_server.h b/src/admin/admin_server.h
index 1d5cbec55f..08877a8edc 100644
--- a/src/admin/admin_server.h
+++ b/src/admin/admin_server.h
@@ -67,3 +67,6 @@ int adminServerSetClientLimits(virNetServerPtr srv,
virTypedParameterPtr params,
int nparams,
unsigned int flags);
+
+int adminServerUpdateTlsFiles(virNetServerPtr srv,
+ unsigned int flags);
diff --git a/src/admin/libvirt-admin.c b/src/admin/libvirt-admin.c
index a8592ebfd3..835b5560d2 100644
--- a/src/admin/libvirt-admin.c
+++ b/src/admin/libvirt-admin.c
@@ -1078,6 +1078,36 @@ virAdmServerSetClientLimits(virAdmServerPtr srv,
return ret;
}
+/**
+ * virAdmServerUpdateTlsFiles:
+ * @srv: a valid server object reference
+ * @flags: extra flags; not used yet, so callers should always pass 0
+ *
+ * Notify server to update tls file, such as cacert, cacrl, server cert / key.
+ *
+ * Returns 0 if the TLS files have been updated successfully or -1 in case of
an
+ * error.
+ */
+int
+virAdmServerUpdateTlsFiles(virAdmServerPtr srv,
+ unsigned int flags)
+{
+int ret = -1;
+
+VIR_DEBUG("srv=%p, flags=0x%x", srv, flags);
+virResetLastError();
+
+virCheckAdmServerGoto(srv, error);
+
+if ((ret = remoteAdminServerUpdateTlsFiles(srv, flags)) < 0)
+goto error;
+
+return ret;
+ error:
+virDispatchError(NULL);
+return ret;
+}
+
/**
* virAdmConnectGetLoggingOutputs:
* @conn: pointer to an active admin connection
diff --git a/src/admin/libvirt_admin_private.syms
b/src/admin/libvirt_admin_private.syms
index 9526412de8..157a45341e 100644
--- a/src/admin/libvirt_admin_private.syms
+++ b/src/admin/libvirt_admin_private.syms
@@ -31,6 +31,7 @@ xdr_admin_server_lookup_client_args;
xdr_admin_server_lookup_client_ret;
xdr_admin_server_set_client_limits_args;
xdr_admin_server_set_threadpool_parameters_args;
+xdr_admin_server_update_tls_files_args;
# datatypes.h
virAdmClientClass;
diff --git a/src/admin/libvirt_admin_public.syms
b/src/admin/libvirt_admin_public.syms
index 9a3f843780..8126973e5b 100644
--- a/src/admin/libvirt_admin_public.syms
+++ b/src/admin/libvirt_admin_public.syms
@@ -38,6 +38,7 @@ LIBVIRT_ADMIN_2.0.0 {
virAdmClientClose;
virAdmServerGetClientLimits;
virAdmServerSetClientLimits;
+virAdmServerUpdateTlsFiles;
};
LIBVIRT_ADMIN_3.0.0 {
--
2.23.0.windows.1
[PATCHv2 5/5] docs: update virt-admin.rst for server-update-tls
Update the manpage for the 'server-update-tls' command --- docs/manpages/virt-admin.rst | 16 1 file changed, 16 insertions(+) diff --git a/docs/manpages/virt-admin.rst b/docs/manpages/virt-admin.rst index 51c3d3917e..3e0d127790 100644 --- a/docs/manpages/virt-admin.rst +++ b/docs/manpages/virt-admin.rst @@ -442,6 +442,22 @@ Set new client-related limits on *server*. *--max-clients*. +server-update-tls +- + +**Syntax:** + +.. code-block:: + + server-update-tls server + +Update tls context on *server*. + +- *server* + + Available servers on a daemon. Currently only supports 'libvirtd'. + + CLIENT COMMANDS === -- 2.23.0.windows.1
[PATCH 3/6] admin: Introduce virAdmServerUpdateTlsFiles
The server needs to use CA certificate, CRL, server certificate/key to
complete the TLS handshake. If these files change, we need to restart
libvirtd for them to take effect. This API can update the TLS context
without restarting libvirtd.
---
include/libvirt/libvirt-admin.h | 4
src/admin/admin_protocol.x | 13 ++-
src/admin/admin_server.c | 13 +++
src/admin/admin_server.h | 4
src/admin/libvirt-admin.c| 34
src/admin/libvirt_admin_private.syms | 1 +
src/admin/libvirt_admin_public.syms | 1 +
7 files changed, 69 insertions(+), 1 deletion(-)
diff --git a/include/libvirt/libvirt-admin.h b/include/libvirt/libvirt-admin.h
index 3edc044490..6e38261129 100644
--- a/include/libvirt/libvirt-admin.h
+++ b/include/libvirt/libvirt-admin.h
@@ -410,6 +410,10 @@ int virAdmServerSetClientLimits(virAdmServerPtr srv,
int nparams,
unsigned int flags);
+int virAdmServerUpdateTlsFiles(virAdmServerPtr srv,
+ unsigned int filetypes,
+ unsigned int flags);
+
int virAdmConnectGetLoggingOutputs(virAdmConnectPtr conn,
char **outputs,
unsigned int flags);
diff --git a/src/admin/admin_protocol.x b/src/admin/admin_protocol.x
index 42e215d23a..0fc8c54c80 100644
--- a/src/admin/admin_protocol.x
+++ b/src/admin/admin_protocol.x
@@ -181,6 +181,12 @@ struct admin_server_set_client_limits_args {
unsigned int flags;
};
+struct admin_server_update_tls_files_args {
+admin_nonnull_server srv;
+unsigned int filetypes;
+unsigned int flags;
+};
+
struct admin_connect_get_logging_outputs_args {
unsigned int flags;
};
@@ -314,5 +320,10 @@ enum admin_procedure {
/**
* @generate: both
*/
-ADMIN_PROC_CONNECT_SET_LOGGING_FILTERS = 17
+ADMIN_PROC_CONNECT_SET_LOGGING_FILTERS = 17,
+
+/**
+ * @generate: both
+ */
+ADMIN_PROC_SERVER_UPDATE_TLS_FILES = 18
};
diff --git a/src/admin/admin_server.c b/src/admin/admin_server.c
index ba87f701c3..558913367b 100644
--- a/src/admin/admin_server.c
+++ b/src/admin/admin_server.c
@@ -367,3 +367,16 @@ adminServerSetClientLimits(virNetServerPtr srv,
return 0;
}
+
+int
+adminServerUpdateTlsFiles(virNetServerPtr srv,
+ unsigned int filetypes,
+ unsigned int flags)
+{
+virCheckFlags(0, -1);
+
+if (virNetServerUpdateTlsFiles(srv, filetypes) < 0)
+return -1;
+
+return 0;
+}
diff --git a/src/admin/admin_server.h b/src/admin/admin_server.h
index 1d5cbec55f..bd355017f2 100644
--- a/src/admin/admin_server.h
+++ b/src/admin/admin_server.h
@@ -67,3 +67,7 @@ int adminServerSetClientLimits(virNetServerPtr srv,
virTypedParameterPtr params,
int nparams,
unsigned int flags);
+
+int adminServerUpdateTlsFiles(virNetServerPtr srv,
+ unsigned int filetypes,
+ unsigned int flags);
diff --git a/src/admin/libvirt-admin.c b/src/admin/libvirt-admin.c
index 4099a54854..f3f92ed91c 100644
--- a/src/admin/libvirt-admin.c
+++ b/src/admin/libvirt-admin.c
@@ -1082,6 +1082,40 @@ virAdmServerSetClientLimits(virAdmServerPtr srv,
return ret;
}
+/**
+ * virAdmServerUpdateTlsFiles:
+ * @srv: a valid server object reference
+ * @filetypes: bitwise-OR of virServerTlsFiletype
+ * @flags: extra flags; not used yet, so callers should always pass 0
+ *
+ * Notify server to update tls file, such as cacert, cacrl, server cert / key.
+ * Mark the files that need to be updated by the @filetypes parameter.
+ * See virServerTlsFiletype for detailed description of accepted filetypes.
+ *
+ * Returns 0 if the TLS files have been updated successfully or -1 in case of
an
+ * error.
+ */
+int
+virAdmServerUpdateTlsFiles(virAdmServerPtr srv,
+ unsigned int filetypes,
+ unsigned int flags)
+{
+int ret = -1;
+
+VIR_DEBUG("srv=%p, filetypes=%u, flags=0x%x", srv, filetypes, flags);
+virResetLastError();
+
+virCheckAdmServerGoto(srv, error);
+
+if ((ret = remoteAdminServerUpdateTlsFiles(srv, filetypes, flags)) < 0)
+goto error;
+
+return ret;
+ error:
+virDispatchError(NULL);
+return ret;
+}
+
/**
* virAdmConnectGetLoggingOutputs:
* @conn: pointer to an active admin connection
diff --git a/src/admin/libvirt_admin_private.syms
b/src/admin/libvirt_admin_private.syms
index 9526412de8..157a45341e 100644
--- a/src/admin/libvirt_admin_private.syms
+++ b/src/admin/libvirt_admin_private.syms
@@ -31,6 +31,7 @@ xdr_admin_server_lookup_client_args;
xdr_admin_server_lookup_client_ret;
xdr_admin_server_set_client_limits_args;
xdr_admin_server_set_threadpool_parameters_args;
[PATCH 0/6] update tls files without restarting libvirtd
When a client wants to establish a TLS connection with libvirtd, a CRL file， CA cert and server cert/key are used. Right now, if these files are changed, you must restart libvirtd to make them take effect. The restart behavior of libvirtd will cause clients connecting with libvirtd to fail. In a server cluster, these files, mostly the CRL, may be updated quite frequently dueto the large amount of certificates. If the new file does not take effect in time, there are security risks. So you may need to restart libvirtd frequently to make the CRL etc. take effect in time. However, frequent restarts will affect the reliability of cluster virtual machine management(such as openstack) services. These patches add a virt-admin command to update the tls related files *online*. Zhang Bo (6): virnettlscontext: refactoring virNetTLSContextLoadCredentials virnetserver: Introduce virNetServerUpdateTlsFiles admin: Introduce virAdmServerUpdateTlsFiles admin: support server cert update mode virt-admin: Introduce command srv-update-tls docs: update virt-admin.rst for server-update-tls docs/manpages/virt-admin.rst | 21 include/libvirt/libvirt-admin.h | 26 src/admin/admin_protocol.x | 13 +- src/admin/admin_server.c | 8 ++ src/admin/admin_server.h | 4 + src/admin/libvirt-admin.c| 39 ++ src/admin/libvirt_admin_private.syms | 1 + src/admin/libvirt_admin_public.syms | 1 + src/libvirt_remote.syms | 1 + src/rpc/virnetserver.c | 81 src/rpc/virnetserver.h | 4 + src/rpc/virnetserverclient.c | 4 + src/rpc/virnettlscontext.c | 179 +++ src/rpc/virnettlscontext.h | 3 + tools/virt-admin.c | 88 + 15 files changed, 419 insertions(+), 54 deletions(-) -- 2.23.0.windows.1
[PATCH 4/6] admin: support server cert update mode
virAdmServerUpdateTlsFiles:
@flags specifies how to update server cert/key in tls service.
Two modes are currently supported: append mode and clear mode, means
whether to clear the original cert then add the new one, or just append
to the original one.
---
include/libvirt/libvirt-admin.h | 14 ++
src/admin/admin_server.c| 7 +--
src/admin/libvirt-admin.c | 7 ++-
src/rpc/virnetserver.c | 17 +
src/rpc/virnetserver.h | 3 ++-
src/rpc/virnettlscontext.c | 7 +--
src/rpc/virnettlscontext.h | 3 ++-
7 files changed, 43 insertions(+), 15 deletions(-)
diff --git a/include/libvirt/libvirt-admin.h b/include/libvirt/libvirt-admin.h
index 6e38261129..dfdd81ae83 100644
--- a/include/libvirt/libvirt-admin.h
+++ b/include/libvirt/libvirt-admin.h
@@ -392,6 +392,20 @@ int virAdmClientClose(virAdmClientPtr client, unsigned int
flags);
# define VIR_SERVER_CLIENTS_UNAUTH_CURRENT "nclients_unauth"
+typedef enum {
+/* free old credentials and then set new tls context.
+ */
+VIR_TLS_UPDATE_CLEAR = 0,
+
+/* do not clear original certificates and keys.
+ */
+VIR_TLS_UPDATE_APPEND = 1,
+
+/* boundary value for flag check (unreachable).
+ */
+VIR_TLS_UPDATE_FLAG_MAX = 2,
+} virServerTlsUpdateFlag;
+
/* tls related filetype flags. */
typedef enum {
VIR_TLS_FILE_TYPE_CA_CERT = (1U << 0),
diff --git a/src/admin/admin_server.c b/src/admin/admin_server.c
index 558913367b..43c7e00d90 100644
--- a/src/admin/admin_server.c
+++ b/src/admin/admin_server.c
@@ -373,10 +373,5 @@ adminServerUpdateTlsFiles(virNetServerPtr srv,
unsigned int filetypes,
unsigned int flags)
{
-virCheckFlags(0, -1);
-
-if (virNetServerUpdateTlsFiles(srv, filetypes) < 0)
-return -1;
-
-return 0;
+return virNetServerUpdateTlsFiles(srv, filetypes, flags);
}
diff --git a/src/admin/libvirt-admin.c b/src/admin/libvirt-admin.c
index f3f92ed91c..b6ba72b577 100644
--- a/src/admin/libvirt-admin.c
+++ b/src/admin/libvirt-admin.c
@@ -1086,12 +1086,17 @@ virAdmServerSetClientLimits(virAdmServerPtr srv,
* virAdmServerUpdateTlsFiles:
* @srv: a valid server object reference
* @filetypes: bitwise-OR of virServerTlsFiletype
- * @flags: extra flags; not used yet, so callers should always pass 0
+ * @flags: mode that specifies the update method
*
* Notify server to update tls file, such as cacert, cacrl, server cert / key.
* Mark the files that need to be updated by the @filetypes parameter.
* See virServerTlsFiletype for detailed description of accepted filetypes.
*
+ * @flags specifies how to update server cert/key in tls service,
+ * and is either the value VIR_TLS_UPDATE_APPEND, or VIR_TLS_UPDATE_CLEAR.
+ * The default value is VIR_TLS_UPDATE_CLEAR. See virServerTlsUpdateFlag for
+ * detailed description.
+ *
* Returns 0 if the TLS files have been updated successfully or -1 in case of
an
* error.
*/
diff --git a/src/rpc/virnetserver.c b/src/rpc/virnetserver.c
index 65ec677d0a..72c4d37bc6 100644
--- a/src/rpc/virnetserver.c
+++ b/src/rpc/virnetserver.c
@@ -1226,7 +1226,8 @@ virNetServerGetTLSContext(virNetServerPtr srv)
return ctxt;
}
-static int virNetServerUpdateTlsFilesCheckParams(unsigned int filetypes)
+static int virNetServerUpdateTlsFilesCheckParams(unsigned int filetypes,
+ unsigned int flags)
{
bool haveSrvCert = filetypes & VIR_TLS_FILE_TYPE_SERVER_CERT;
bool haveSrvKey = filetypes & VIR_TLS_FILE_TYPE_SERVER_KEY;
@@ -1239,12 +1240,20 @@ static int
virNetServerUpdateTlsFilesCheckParams(unsigned int filetypes)
return -1;
}
+if (flags >= VIR_TLS_UPDATE_FLAG_MAX) {
+virReportError(VIR_ERR_SYSTEM_ERROR,
+ _("don not support flags: %d"),
+ flags);
+return -1;
+}
+
return 0;
}
int
virNetServerUpdateTlsFiles(virNetServerPtr srv,
- unsigned int filetypes)
+ unsigned int filetypes,
+ unsigned int flags)
{
int ret = -1;
#ifndef WITH_GNUTLS
@@ -1254,7 +1263,7 @@ virNetServerUpdateTlsFiles(virNetServerPtr srv,
#else
virNetTLSContextPtr ctxt = NULL;
-if (virNetServerUpdateTlsFilesCheckParams(filetypes))
+if (virNetServerUpdateTlsFilesCheckParams(filetypes, flags))
return -1;
virObjectLock(srv);
@@ -1266,7 +1275,7 @@ virNetServerUpdateTlsFiles(virNetServerPtr srv,
goto cleanup;
}
-if (virNetTLSContextReload(ctxt, filetypes)) {
+if (virNetTLSContextReload(ctxt, filetypes, flags)) {
VIR_ERROR(_("reload server's tls context fail"));
goto cleanup;
}
diff --git a/src/rpc/virnetserver.h b/src/rpc/virnetserver.h
index 99466dd041..1a905aa483 100644
--- a/src/rpc/virnetserver.h
+++
[PATCH 1/6] virnettlscontext: refactoring virNetTLSContextLoadCredentials
Encapsulate the code for setting TLS-related files into functions,
which is convenient for other modules to call.
---
src/rpc/virnettlscontext.c | 135 ++---
1 file changed, 82 insertions(+), 53 deletions(-)
diff --git a/src/rpc/virnettlscontext.c b/src/rpc/virnettlscontext.c
index 44f0dfce77..12811bed78 100644
--- a/src/rpc/virnettlscontext.c
+++ b/src/rpc/virnettlscontext.c
@@ -594,6 +594,85 @@ static int virNetTLSContextSanityCheckCredentials(bool
isServer,
return ret;
}
+static int virNetTLSContextSetCACert(virNetTLSContextPtr ctxt,
+ const char *cacert,
+ bool allowMissing)
+{
+int err;
+if (virNetTLSContextCheckCertFile("CA certificate", cacert, allowMissing)
< 0)
+return -1;
+
+VIR_DEBUG("loading CA cert from %s", cacert);
+err = gnutls_certificate_set_x509_trust_file(ctxt->x509cred,
+ cacert,
+ GNUTLS_X509_FMT_PEM);
+if (err < 0) {
+virReportError(VIR_ERR_SYSTEM_ERROR,
+ _("Unable to set x509 CA certificate: %s: %s"),
+ cacert, gnutls_strerror(err));
+return -1;
+}
+
+return 0;
+}
+
+static int virNetTLSContextSetCACRL(virNetTLSContextPtr ctxt,
+const char *cacrl,
+bool allowMissing)
+{
+int rv, err;
+if ((rv = virNetTLSContextCheckCertFile("CA revocation list", cacrl,
allowMissing)) < 0)
+return -1;
+
+if (rv == 0) {
+VIR_DEBUG("loading CRL from %s", cacrl);
+err = gnutls_certificate_set_x509_crl_file(ctxt->x509cred,
+ cacrl,
+ GNUTLS_X509_FMT_PEM);
+if (err < 0) {
+virReportError(VIR_ERR_SYSTEM_ERROR,
+ _("Unable to set x509 certificate revocation list:
%s: %s"),
+ cacrl, gnutls_strerror(err));
+return -1;
+}
+} else {
+VIR_DEBUG("Skipping non-existent CA CRL %s", cacrl);
+}
+
+return 0;
+}
+
+static int virNetTLSContextSetCertAndKey(virNetTLSContextPtr ctxt,
+ const char *cert,
+ const char *key,
+ bool allowMissing)
+{
+int rv, err;
+if ((rv = virNetTLSContextCheckCertFile("certificate", cert,
allowMissing)) < 0)
+return -1;
+if (rv == 0 &&
+(rv = virNetTLSContextCheckCertFile("private key", key, allowMissing))
< 0)
+return -1;
+
+if (rv == 0) {
+VIR_DEBUG("loading cert and key from %s and %s", cert, key);
+err =
+gnutls_certificate_set_x509_key_file(ctxt->x509cred,
+ cert, key,
+ GNUTLS_X509_FMT_PEM);
+if (err < 0) {
+virReportError(VIR_ERR_SYSTEM_ERROR,
+ _("Unable to set x509 key and certificate: %s, %s:
%s"),
+ key, cert, gnutls_strerror(err));
+return -1;
+}
+} else {
+VIR_DEBUG("Skipping non-existent cert %s key %s on client",
+ cert, key);
+}
+
+return 0;
+}
static int virNetTLSContextLoadCredentials(virNetTLSContextPtr ctxt,
bool isServer,
@@ -602,69 +681,19 @@ static int
virNetTLSContextLoadCredentials(virNetTLSContextPtr ctxt,
const char *cert,
const char *key)
{
-int err;
-
if (cacert && cacert[0] != '\0') {
-if (virNetTLSContextCheckCertFile("CA certificate", cacert, false) < 0)
-return -1;
-
-VIR_DEBUG("loading CA cert from %s", cacert);
-err = gnutls_certificate_set_x509_trust_file(ctxt->x509cred,
- cacert,
- GNUTLS_X509_FMT_PEM);
-if (err < 0) {
-virReportError(VIR_ERR_SYSTEM_ERROR,
- _("Unable to set x509 CA certificate: %s: %s"),
- cacert, gnutls_strerror(err));
+if (virNetTLSContextSetCACert(ctxt, cacert, false))
return -1;
-}
}
if (cacrl && cacrl[0] != '\0') {
-int rv;
-if ((rv = virNetTLSContextCheckCertFile("CA revocation list", cacrl,
true)) < 0)
+if (virNetTLSContextSetCACRL(ctxt, cacrl, true))
return -1;
-
-if (rv == 0) {
-VIR_DEBUG("loading CRL from %s", cacrl);
-err = gnutls_certificate_set_x509_crl_file(ctxt->x509cred,
- cacrl,
[PATCH 5/6] virt-admin: Introduce command srv-update-tls
wire-up virAdmServerUpdateTlsFiles API into virt-admin client.
---
tools/virt-admin.c | 88 ++
1 file changed, 88 insertions(+)
diff --git a/tools/virt-admin.c b/tools/virt-admin.c
index 32edfe5757..85235ae03d 100644
--- a/tools/virt-admin.c
+++ b/tools/virt-admin.c
@@ -957,6 +957,84 @@ cmdSrvClientsSet(vshControl *ctl, const vshCmd *cmd)
goto cleanup;
}
+/*
+ * Command srv-update-tls
+ *
+ */
+static const vshCmdInfo info_srv_update_tls_file[] = {
+{.name = "help",
+ .data = N_("notify server to update TLS related files online.")
+},
+{.name = "desc",
+ .data = N_("notify server to update the CA cert, "
+"CA CRL, server cert / key without restarts. "
+"See OPTIONS for currently supported attributes.")
+},
+{.name = NULL}
+};
+
+static const vshCmdOptDef opts_srv_update_tls_file[] = {
+{.name = "server",
+ .type = VSH_OT_DATA,
+ .flags = VSH_OFLAG_REQ,
+ .help = N_("Available servers on a daemon. "
+"Currently only supports 'libvirtd'.")
+},
+{.name = "filetypes",
+ .type = VSH_OT_INT,
+ .flags = VSH_OFLAG_REQ,
+ .help = N_("filetypes that need to be updated. "
+"bitwise-OR of tls filetypes flags.\n"
+" parameter Description:\n"
+"--filetypes 1 ===> cacert\n"
+"--filetypes 2 ===> cacrl\n"
+"--filetypes 4 ===> server-cert\n"
+"--filetypes 8 ===> server-key\n"
+" or a combination of several values. eg:\n"
+"--filetypes 3 ===> cacert | cacrl\n"
+" notice:\n"
+"server cert and key must be updated together.\n")
+},
+{.name = NULL}
+};
+
+static bool
+cmdSrvUpdateTlsFiles(vshControl *ctl, const vshCmd *cmd)
+{
+bool ret = false;
+const char *srvname = NULL;
+unsigned int filetypes;
+
+virAdmServerPtr srv = NULL;
+vshAdmControlPtr priv = ctl->privData;
+
+if (vshCommandOptStringReq(ctl, cmd, "server", ) < 0)
+return false;
+
+if (vshCommandOptUInt(ctl, cmd, "filetypes", ) < 0)
+return false;
+
+if (filetypes == 0) {
+vshError(ctl, "%s", _("filetypes can not be 0."));
+goto cleanup;
+}
+
+if (!(srv = virAdmConnectLookupServer(priv->conn, srvname, 0)))
+goto cleanup;
+
+if (virAdmServerUpdateTlsFiles(srv, filetypes, VIR_TLS_UPDATE_CLEAR) < 0) {
+vshError(ctl, "%s", _("Unable to update server's tls related files."));
+goto cleanup;
+}
+
+ret = true;
+vshPrint(ctl, "update tls related files succeed\n");
+
+ cleanup:
+virAdmServerFree(srv);
+return ret;
+}
+
/* --
* Command daemon-log-filters
* --
@@ -1436,6 +1514,16 @@ static const vshCmdDef managementCmds[] = {
.info = info_srv_clients_set,
.flags = 0
},
+{.name = "srv-update-tls",
+ .flags = VSH_CMD_FLAG_ALIAS,
+ .alias = "server-update-tls"
+},
+{.name = "server-update-tls",
+ .handler = cmdSrvUpdateTlsFiles,
+ .opts = opts_srv_update_tls_file,
+ .info = info_srv_update_tls_file,
+ .flags = 0
+},
{.name = "daemon-log-filters",
.handler = cmdDaemonLogFilters,
.opts = opts_daemon_log_filters,
--
2.23.0.windows.1
[PATCH 2/6] virnetserver: Introduce virNetServerUpdateTlsFiles
Add an API to update server's tls context before admin method can be
introduced.
---
include/libvirt/libvirt-admin.h | 8
src/libvirt_remote.syms | 1 +
src/rpc/virnetserver.c | 72 +
src/rpc/virnetserver.h | 3 ++
src/rpc/virnetserverclient.c| 4 ++
src/rpc/virnettlscontext.c | 41 +++
src/rpc/virnettlscontext.h | 2 +
7 files changed, 131 insertions(+)
diff --git a/include/libvirt/libvirt-admin.h b/include/libvirt/libvirt-admin.h
index abf2792926..3edc044490 100644
--- a/include/libvirt/libvirt-admin.h
+++ b/include/libvirt/libvirt-admin.h
@@ -392,6 +392,14 @@ int virAdmClientClose(virAdmClientPtr client, unsigned int
flags);
# define VIR_SERVER_CLIENTS_UNAUTH_CURRENT "nclients_unauth"
+/* tls related filetype flags. */
+typedef enum {
+VIR_TLS_FILE_TYPE_CA_CERT = (1U << 0),
+VIR_TLS_FILE_TYPE_CA_CRL = (1U << 1),
+VIR_TLS_FILE_TYPE_SERVER_CERT = (1U << 2),
+VIR_TLS_FILE_TYPE_SERVER_KEY = (1U << 3),
+} virServerTlsFiletype;
+
int virAdmServerGetClientLimits(virAdmServerPtr srv,
virTypedParameterPtr *params,
int *nparams,
diff --git a/src/libvirt_remote.syms b/src/libvirt_remote.syms
index 0493467f46..0018a0c41d 100644
--- a/src/libvirt_remote.syms
+++ b/src/libvirt_remote.syms
@@ -137,6 +137,7 @@ virNetServerSetClientLimits;
virNetServerSetThreadPoolParameters;
virNetServerSetTLSContext;
virNetServerUpdateServices;
+virNetServerUpdateTlsFiles;
# rpc/virnetserverclient.h
diff --git a/src/rpc/virnetserver.c b/src/rpc/virnetserver.c
index c87dade1a8..65ec677d0a 100644
--- a/src/rpc/virnetserver.c
+++ b/src/rpc/virnetserver.c
@@ -1207,3 +1207,75 @@ virNetServerSetClientLimits(virNetServerPtr srv,
virObjectUnlock(srv);
return ret;
}
+
+static virNetTLSContextPtr
+virNetServerGetTLSContext(virNetServerPtr srv)
+{
+size_t i;
+virNetTLSContextPtr ctxt = NULL;
+virNetServerServicePtr svc = NULL;
+
+/* find svcTLS from srv, get svcTLS->tls */
+for (i = 0; i < srv->nservices; i++) {
+svc = srv->services[i];
+ctxt = virNetServerServiceGetTLSContext(svc);
+if (ctxt != NULL)
+break;
+}
+
+return ctxt;
+}
+
+static int virNetServerUpdateTlsFilesCheckParams(unsigned int filetypes)
+{
+bool haveSrvCert = filetypes & VIR_TLS_FILE_TYPE_SERVER_CERT;
+bool haveSrvKey = filetypes & VIR_TLS_FILE_TYPE_SERVER_KEY;
+
+if ((haveSrvCert && !haveSrvKey) ||
+(!haveSrvCert && haveSrvKey)) {
+virReportError(VIR_ERR_SYSTEM_ERROR,
+ _("server cert/key must be updated together. "
+ "filetypes: %d"), filetypes);
+return -1;
+}
+
+return 0;
+}
+
+int
+virNetServerUpdateTlsFiles(virNetServerPtr srv,
+ unsigned int filetypes)
+{
+int ret = -1;
+#ifndef WITH_GNUTLS
+virReportError(VIR_ERR_SYSTEM_ERROR,
+ _("Don't support GNUTLS, can't to update filetypes: %d"),
+ filetypes);
+#else
+virNetTLSContextPtr ctxt = NULL;
+
+if (virNetServerUpdateTlsFilesCheckParams(filetypes))
+return -1;
+
+virObjectLock(srv);
+
+ctxt = virNetServerGetTLSContext(srv);
+if (!ctxt) {
+VIR_ERROR(_("no tls svc found, can't to update filetypes: %d"),
+ filetypes);
+goto cleanup;
+}
+
+if (virNetTLSContextReload(ctxt, filetypes)) {
+VIR_ERROR(_("reload server's tls context fail"));
+goto cleanup;
+}
+
+VIR_INFO("update all tls files complete, filetypes: %d", filetypes);
+ret = 0;
+
+ cleanup:
+virObjectUnlock(srv);
+#endif
+return ret;
+}
diff --git a/src/rpc/virnetserver.h b/src/rpc/virnetserver.h
index 260c99b22d..99466dd041 100644
--- a/src/rpc/virnetserver.h
+++ b/src/rpc/virnetserver.h
@@ -133,3 +133,6 @@ size_t virNetServerGetCurrentUnauthClients(virNetServerPtr
srv);
int virNetServerSetClientLimits(virNetServerPtr srv,
long long int maxClients,
long long int maxClientsUnauth);
+
+int virNetServerUpdateTlsFiles(virNetServerPtr srv,
+ unsigned int filetypes);
diff --git a/src/rpc/virnetserverclient.c b/src/rpc/virnetserverclient.c
index 67b3bf9531..f0952cadde 100644
--- a/src/rpc/virnetserverclient.c
+++ b/src/rpc/virnetserverclient.c
@@ -1117,7 +1117,9 @@ int virNetServerClientInit(virNetServerClientPtr client)
client->tls);
/* Begin the TLS handshake. */
+virObjectLock(client->tlsCtxt);
ret = virNetTLSSessionHandshake(client->tls);
+virObjectUnlock(client->tlsCtxt);
if (ret == 0) {
/* Unlikely, but ... Next step is to check the certificate. */
if (virNetServerClientCheckAccess(client) < 0)
@@
[PATCH 6/6] docs: update virt-admin.rst for server-update-tls
Update the manpage for the 'server-update-tls' command --- docs/manpages/virt-admin.rst | 21 + 1 file changed, 21 insertions(+) diff --git a/docs/manpages/virt-admin.rst b/docs/manpages/virt-admin.rst index 51c3d3917e..e19d1f1577 100644 --- a/docs/manpages/virt-admin.rst +++ b/docs/manpages/virt-admin.rst @@ -442,6 +442,27 @@ Set new client-related limits on *server*. *--max-clients*. +server-update-tls +- + +**Syntax:** + +.. code-block:: + + server-update-tls server [--filetypes types] + +Update tls context on *server*. + +- *server* + + Available servers on a daemon. Currently only supports 'libvirtd'. + +- *--filetypes* + + Indicate which TLS related files need to be updated, such as CA cert, CA CRL, + server cert/key. ``types`` is bitwise-OR of tls related files. + + CLIENT COMMANDS === -- 2.23.0.windows.1
Re: [libvirt] ambiguous ret of qemuDomainDetachVirtioDiskDevice
On 2015/7/30 17:41, zhang bo wrote:
On 2015/7/28 16:33, Ján Tomko wrote:
On Tue, Jul 28, 2015 at 04:25:13PM +0800, zhang bo wrote:
static int
qemuDomainDetachVirtioDiskDevice(virQEMUDriverPtr driver,
virDomainObjPtr vm,
virDomainDiskDefPtr detach)
{
...
rc = qemuDomainWaitForDeviceRemoval(vm);
if (rc == 0 || rc == 1)
ret = qemuDomainRemoveDiskDevice(driver, vm, detach);
else
ret = 0; /*the return value of 2 is dismissed here, which refers
to ETIMEOUT.*/
}
If it timeouts when qemu tries to del the device, the return value would be
modified from 2 to 0 in
function qemuDomainDetachVirtioDiskDevice(), which means that, the users
would be misleaded that
the device has been deleted, however, the device maybe probably failed to
be detached after timeout and
still in use.
This is intentional and documented:
http://libvirt.org/html/libvirt-libvirt-domain.html#virDomainDetachDeviceFlags
Unplugging a disk requires guest cooperation, so the best we can do is
ask qemu to unplug it and wait for a while.
That is to say, the function qemuDomainDetachVirtioDiskDevice()'s return
value is ambiguous when it's 0,
maybe successful, or timeout. Will it be better to pass ETIMEOUT to user?
or any other advises? for example,
let users themselves dumpxml the guest to check whether the device has been
actually detached or not?
Either dump the XML, or wait for the VIR_DOMAIN_EVENT_ID_DEVICE_REMOVED
event, as the API documentation suggests.
Jan
It seems to have fixed the problem by dumping the XML or wait for the
DEVICE_REMOVED event. However, it seems to
make nova or other apps to do more checking work, they need to dump XML or
wait the event even if the device has
already been successfully removed. which is unnecessary.
I think, it's better to return ETIMEOUT and let nova to dumpxml or wait the
event at this situation, rather than always
doing that job.
It maybe a better design, what's your opinion?
After thinking twice, it's an async job, thus returning 0 is acceptable, right?
--
Oscar
oscar.zhan...@huawei.com
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] ambiguous ret of qemuDomainDetachVirtioDiskDevice
On 2015/7/28 16:33, Ján Tomko wrote:
On Tue, Jul 28, 2015 at 04:25:13PM +0800, zhang bo wrote:
static int
qemuDomainDetachVirtioDiskDevice(virQEMUDriverPtr driver,
virDomainObjPtr vm,
virDomainDiskDefPtr detach)
{
...
rc = qemuDomainWaitForDeviceRemoval(vm);
if (rc == 0 || rc == 1)
ret = qemuDomainRemoveDiskDevice(driver, vm, detach);
else
ret = 0; /*the return value of 2 is dismissed here, which refers to
ETIMEOUT.*/
}
If it timeouts when qemu tries to del the device, the return value would be
modified from 2 to 0 in
function qemuDomainDetachVirtioDiskDevice(), which means that, the users
would be misleaded that
the device has been deleted, however, the device maybe probably failed to be
detached after timeout and
still in use.
This is intentional and documented:
http://libvirt.org/html/libvirt-libvirt-domain.html#virDomainDetachDeviceFlags
Unplugging a disk requires guest cooperation, so the best we can do is
ask qemu to unplug it and wait for a while.
That is to say, the function qemuDomainDetachVirtioDiskDevice()'s return
value is ambiguous when it's 0,
maybe successful, or timeout. Will it be better to pass ETIMEOUT to user? or
any other advises? for example,
let users themselves dumpxml the guest to check whether the device has been
actually detached or not?
Either dump the XML, or wait for the VIR_DOMAIN_EVENT_ID_DEVICE_REMOVED
event, as the API documentation suggests.
Jan
It seems to have fixed the problem by dumping the XML or wait for the
DEVICE_REMOVED event. However, it seems to
make nova or other apps to do more checking work, they need to dump XML or wait
the event even if the device has
already been successfully removed. which is unnecessary.
I think, it's better to return ETIMEOUT and let nova to dumpxml or wait the
event at this situation, rather than always
doing that job.
It maybe a better design, what's your opinion?
--
Oscar
oscar.zhan...@huawei.com
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] ambiguous ret of qemuDomainDetachVirtioDiskDevice
static int
qemuDomainDetachVirtioDiskDevice(virQEMUDriverPtr driver,
virDomainObjPtr vm,
virDomainDiskDefPtr detach)
{
...
rc = qemuDomainWaitForDeviceRemoval(vm);
if (rc == 0 || rc == 1)
ret = qemuDomainRemoveDiskDevice(driver, vm, detach);
else
ret = 0; /*the return value of 2 is dismissed here, which refers to
ETIMEOUT.*/
}
If it timeouts when qemu tries to del the device, the return value would be
modified from 2 to 0 in
function qemuDomainDetachVirtioDiskDevice(), which means that, the users would
be misleaded that
the device has been deleted, however, the device maybe probably failed to be
detached after timeout and
still in use.
That is to say, the function qemuDomainDetachVirtioDiskDevice()'s return value
is ambiguous when it's 0,
maybe successful, or timeout. Will it be better to pass ETIMEOUT to user? or
any other advises? for example,
let users themselves dumpxml the guest to check whether the device has been
actually detached or not?
--
Oscar
oscar.zhan...@huawei.com
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] shall libvirtd validate guest's name ?
linux-ZyvZnF:~ # virsh list --all IdName State - redhat7;reboot shut off - oscar-vm-5 shut off As shown above, 1 we use command virsh define a.xml to define a guest with a name containing ';', that's 'redhat7;reboot' 2 then we start the guest: virsh start redhat7;reboot 3 shell consider the command as a) run virsh start redhat7, failed b) run reboot, to reboot the host And *the host get rebooted*. shall libvirtd do the guest-name-validation work? Or other suggustions? -- Oscar oscar.zhan...@huawei.com -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCHv3] qemu: add a check for slot and base when build dimm address
On 2015/6/15 20:33, Luyao Huang wrote:
When hot-plug a memory device, we don't check if there
is a memory device have the same address with the memory device
we want hot-pluged. Qemu forbid use/hot-plug 2 memory device
with same slot or the same base(qemu side this elemnt named addr).
+for (i = 0; i def-nmems; i++) {
+ virDomainMemoryDefPtr tmp = def-mems[i];
+
+ if (tmp == mem ||
+ tmp-info.type != VIR_DOMAIN_DEVICE_ADDRESS_TYPE_DIMM)
+ continue;
+
+ if (mem-info.addr.dimm.slot == tmp-info.addr.dimm.slot) {
+ virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
+_(memory device slot '%u' is already being
+ used by another memory device),
+mem-info.addr.dimm.slot);
+ return true;
+ }
+
+ if (mem-info.addr.dimm.base != 0 tmp-info.addr.dimm.base != 0
+ mem-info.addr.dimm.base == tmp-info.addr.dimm.base) {
The logic here equals:
if (mem-info.addr.dimm.base != 0
mem-info.addr.dimm.base == tmp-info.addr.dimm.base) {
will it be better like this?
+ virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
+_(memory device base '0x%llx' is already being
+ used by another memory device),
+mem-info.addr.dimm.base);
+ return true;
+ }
+}
+
+return false;
+}
+
char *
qemuBuildMemoryDeviceStr(virDomainMemoryDefPtr mem,
virDomainDefPtr def,
@@ -4993,6 +5027,9 @@ qemuBuildMemoryDeviceStr(virDomainMemoryDefPtr mem,
mem-targetNode, mem-info.alias, mem-info.alias);
if (mem-info.type == VIR_DOMAIN_DEVICE_ADDRESS_TYPE_DIMM) {
+if (qemuCheckMemoryDimmConflict(def, mem))
+return NULL;
+
virBufferAsprintf(buf, ,slot=%d, mem-info.addr.dimm.slot);
virBufferAsprintf(buf, ,addr=%llu, mem-info.addr.dimm.base);
}
--
Oscar
oscar.zhan...@huawei.com
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] qemu: update netdevs of the same mac addrs correctly
On 2015/6/15 8:27, zhang bo wrote: ping On 2015/6/8 16:25, zhang bo wrote: If a guest has multiple network devices with the same MAC address, when we online update the second device, libvirtd always updates the first one. commit def31e4c forgot to fix the online updating scenario. We need to use virDomainNetFindIdx() to find the correct network device. Signed-off-by: Zhou Yimin zhouyi...@huawei.com Signed-off-by: Zhang Bo oscar.zhan...@huawei.com --- src/qemu/qemu_hotplug.c | 23 +++ 1 file changed, 7 insertions(+), 16 deletions(-) Sorry for mistakenly ping :) ACKed at https://www.redhat.com/archives/libvir-list/2015-June/msg00585.html -- Oscar oscar.zhan...@huawei.com -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] qemu: update netdevs of the same mac addrs correctly
ping
On 2015/6/8 16:25, zhang bo wrote:
If a guest has multiple network devices with the same MAC address,
when we online update the second device, libvirtd always updates
the first one.
commit def31e4c forgot to fix the online updating scenario. We need to
use virDomainNetFindIdx() to find the correct network device.
Signed-off-by: Zhou Yimin zhouyi...@huawei.com
Signed-off-by: Zhang Bo oscar.zhan...@huawei.com
---
src/qemu/qemu_hotplug.c | 23 +++
1 file changed, 7 insertions(+), 16 deletions(-)
diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c
index 94ebe35..d455bd6 100644
--- a/src/qemu/qemu_hotplug.c
+++ b/src/qemu/qemu_hotplug.c
@@ -2052,20 +2052,6 @@ int qemuDomainAttachHostDevice(virConnectPtr conn,
return -1;
}
-static virDomainNetDefPtr *qemuDomainFindNet(virDomainObjPtr vm,
- virDomainNetDefPtr dev)
-{
-size_t i;
-
-for (i = 0; i vm-def-nnets; i++) {
-if (virMacAddrCmp(vm-def-nets[i]-mac, dev-mac) == 0)
-return vm-def-nets[i];
-}
-
-return NULL;
-}
-
-
static int
qemuDomainChangeNetBridge(virDomainObjPtr vm,
virDomainNetDefPtr olddev,
@@ -2195,7 +2181,7 @@ qemuDomainChangeNet(virQEMUDriverPtr driver,
virDomainDeviceDefPtr dev)
{
virDomainNetDefPtr newdev = dev-data.net;
-virDomainNetDefPtr *devslot = qemuDomainFindNet(vm, newdev);
+virDomainNetDefPtr *devslot = NULL;
virDomainNetDefPtr olddev;
int oldType, newType;
bool needReconnect = false;
@@ -2205,8 +2191,13 @@ qemuDomainChangeNet(virQEMUDriverPtr driver,
bool needReplaceDevDef = false;
bool needBandwidthSet = false;
int ret = -1;
+int changeidx = -1;
+
+if ((changeidx = virDomainNetFindIdx(vm-def, newdev)) 0)
+goto cleanup;
+devslot = vm-def-nets[changeidx];
-if (!devslot || !(olddev = *devslot)) {
+if (!(olddev = *devslot)) {
virReportError(VIR_ERR_OPERATION_FAILED, %s,
_(cannot find existing network device to modify));
goto cleanup;
--
Oscar
oscar.zhan...@huawei.com
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [RFC] get guest OS infos
On 2015/6/11 21:54, Daniel P. Berrange wrote: On Thu, Jun 11, 2015 at 08:47:12AM -0500, Dennis Jenkins wrote: On Thu, Jun 11, 2015 at 3:51 AM, Daniel P. Berrange berra...@redhat.com wrote: On Thu, Jun 11, 2015 at 09:17:30AM +0100, Daniel P. Berrange wrote: On Thu, Jun 11, 2015 at 01:51:33PM +0800, zhang bo wrote: Different OSes have different capabilities and behaviors sometimes. We have to distinguish them then. For example, our clients want to send NMI interrupts to certain guests(eg.Linux distributions), but not others(eg.Windows guests). They want to acquire the list below: guest1: RHEL 7 guest2: RHEL 7 guest3: Ubuntu 12 guest4: Ubuntu 13 guest5: Windows 7 .. AFAIK, neither libvirt nor openstack, nor qemu, have such capbility of showing these guest OS infos. Libvirt now supports to show host capabilities and driver capability, but not an individual guest OS's capibility. We may refer to http://libvirt.org/formatdomaincaps.html for more information. Hello. I wrote a utility a few years ago to detect which OS is running in each qemu VM under libvirt via memory probing. I have not touched the code in a few years. YMMV. http://pastebin.com/m0mfcK8G FWIW, you can also use libguestfs to analyse the disk of a libvirt guest while it is running, if you libguestfs' use readonly mode Regards, Daniel Great. It seems much better to have Glance and libosinfo together to get guest osinfo, because you don't have to start the guest to get its osinfo, that's attracting. I just have one question: It uses Glance to set os-name, and let libosinfo search its database to get further more infos(I don't know if I'm right here). Is there any possibility that we set a false os-name to image by Glance, for example, the guest is fedora12, but we set it to Ubuntu13 via Glance. and, when would this feature be implemented in Openstack? Thanks. -- Oscar oscar.zhan...@huawei.com -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [RFC] get guest OS infos
On 2015/6/12 17:05, Daniel P. Berrange wrote: On Fri, Jun 12, 2015 at 04:56:17PM +0800, zhang bo wrote: On 2015/6/11 21:54, Daniel P. Berrange wrote: On Thu, Jun 11, 2015 at 08:47:12AM -0500, Dennis Jenkins wrote: On Thu, Jun 11, 2015 at 3:51 AM, Daniel P. Berrange berra...@redhat.com wrote: On Thu, Jun 11, 2015 at 09:17:30AM +0100, Daniel P. Berrange wrote: On Thu, Jun 11, 2015 at 01:51:33PM +0800, zhang bo wrote: Different OSes have different capabilities and behaviors sometimes. We Great. It seems much better to have Glance and libosinfo together to get guest osinfo, because you don't have to start the guest to get its osinfo, that's attracting. I just have one question: It uses Glance to set os-name, and let libosinfo search its database to get further more infos(I don't know if I'm right here). Is there any possibility that we set a false os-name to image by Glance, for example, the guest is fedora12, but we set it to Ubuntu13 via Glance. It of course relies on the user setting the correct operating system name for their image. If the user sets this wrong, then the guest may end up getting the wrong hardware config. and, when would this feature be implemented in Openstack? Thanks. It is under review https://review.openstack.org/#/q/status:open+project:openstack/nova+branch:master+topic:bp/libvirt-hardware-policy-from-libosinfo,n,z NB, this only covers configuration of disk network drivers. In the future we'll extend it to other interesting things Thank you very much! I'd follow there then. -- Oscar oscar.zhan...@huawei.com -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 0/8] logically memory hotplug via guest agent
On 2015/6/10 16:39, Vasiliy Tolstov wrote: 2015-06-10 11:37 GMT+03:00 Daniel P. Berrange berra...@redhat.com: The udev rules are really something the OS vendor should setup, so that it just works I think so, also for vcpu hotplug this also covered by udev. May be we need something to hot remove memory and cpu, because in guest we need offline firstly. In fact ,we also have --guest option for 'virsh sevvcpus' command, which also uses qga commands to do the logical hotplug/unplug jobs, although udev rules seems to cover the vcpu logical hotplug issue. virsh # help setvcpus . --guest modify cpu state in the guest BTW: we didn't see OSes with udev rules for memory-hotplug-event setted by vendors, and adding such rules means that we have to *interfere within the guest*, It seems not a good option. -- Oscar oscar.zhan...@huawei.com -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 0/8] logically memory hotplug via guest agent
On 2015/6/10 13:40, Vasiliy Tolstov wrote: 2015-06-10 5:28 GMT+03:00 zhang bo oscar.zhan...@huawei.com: Thank you for your reply. Before this patch, we needed to manually online memory blocks inside the guest, after dimm memory hotplug for most *nix OSes. (Windows guests automatically get their memory blocks online after hotplugging) That is to say, we need to LOGICALLY hotplug memory after PHYSICAL hotplug. This patch did the LOGICAL part. With this patch, we don't need to get into the guest to manually online them anymore, which is even impossible for most host administrators. As i remember this online step easy can be automate via udev rules. Logically that's true, but adding udev rules means: 1 you have to get into the guest 2 you have to be familar with udev rules. Not convenient enough compared to just calling libvirt API to do so. -- Oscar oscar.zhan...@huawei.com -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 0/8] logically memory hotplug via guest agent
On 2015/6/10 17:31, Daniel P. Berrange wrote: On Wed, Jun 10, 2015 at 10:28:08AM +0100, Daniel P. Berrange wrote: On Wed, Jun 10, 2015 at 05:24:50PM +0800, zhang bo wrote: On 2015/6/10 16:39, Vasiliy Tolstov wrote: 2015-06-10 11:37 GMT+03:00 Daniel P. Berrange berra...@redhat.com: The udev rules are really something the OS vendor should setup, so that it just works I think so, also for vcpu hotplug this also covered by udev. May be we need something to hot remove memory and cpu, because in guest we need offline firstly. In fact ,we also have --guest option for 'virsh sevvcpus' command, which also uses qga commands to do the logical hotplug/unplug jobs, although udev rules seems to cover the vcpu logical hotplug issue. virsh # help setvcpus . --guest modify cpu state in the guest BTW: we didn't see OSes with udev rules for memory-hotplug-event setted by vendors, and adding such rules means that we have to *interfere within the guest*, It seems not a good option. I was suggesting that an RFE be filed with any vendor who doesn't do it to add this capability, not that we add udev rules ourselves. Or actually, it probably is sufficient to just send a patch to the upstream systemd project to add the desired rule to udev. That way all Linux distros will inherit the feature when they update to new udev. Then, here comes the question: how to deal with the guests that are already in use? I think it's better to operate them at the host side without getting into the guest. That's the advantage of qemu-guest-agent, why not take advantage of it? -- Oscar oscar.zhan...@huawei.com -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [RFC] get guest OS infos
Different OSes have different capabilities and behaviors sometimes. We have to
distinguish them then.
For example, our clients want to send NMI interrupts to certain guests(eg.Linux
distributions), but not others(eg.Windows guests).
They want to acquire the list below:
guest1: RHEL 7
guest2: RHEL 7
guest3: Ubuntu 12
guest4: Ubuntu 13
guest5: Windows 7
..
AFAIK, neither libvirt nor openstack, nor qemu, have such capbility of showing
these guest OS infos.
Libvirt now supports to show host capabilities and driver capability, but not
an individual guest OS's capibility. We may refer to
http://libvirt.org/formatdomaincaps.html for more information.
So, what's your opinion on adding such feature in libvirt and qemu?
---
The solution I can see is:
1 add a new qga command in qemu agent, 'guest-get-osinfo', which gets the os
infos by qemu-agent inside the guest.
{ 'command': 'guest-get-osinfo',
'returns': ['GuestOSInfo'] }
{ 'struct': 'GuestOSInfo',
'data': {'distribution': 'GuestOSDistribution',
'version': 'int',
'arch': 'GuestOSArchType'} }
an example Json result:
{return:
{distribution: RHEL,
version: 7,
arch: x86_64
}
}
2 add new helper APIs for that qga command in libvirt.
qemuAgentGetOSInfo()
3 When the guest starts up and its qemu-agent is running, call
qemuAgentGetOSINfo() in libvirt.
4 set the osinfo, which is got at step 3, into the guest's status and config
file.
domainCapabilities
path/usr/bin/qemu-system-x86_64/path
domainkvm/domain
machinepc-i440fx-2.1/machine
archx86_64/arch
distribution
typeRHEL/type
version7/version
/distribution
...
/domainCapabilities
This feature is added into the xml node 'domainCapabilities' here. If there's
any other better choice, please let me know.
--
Oscar
oscar.zhan...@huawei.com
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 0/8] logically memory hotplug via guest agent
Logically memory hotplug via guest agent, by enabling/disabling memory blocks. The corresponding qga commands are: 'guest-get-memory-blocks', 'guest-set-memory-blocks' and 'guest-get-memory-block-info'. detailed flow: 1 get memory block list, each member has 'phy-index', 'online' and 'can-offline' parameters 2 get memory block size, normally 128MB or 256MB for most OSes 3 convert the target memory size to memory block number, and see if there's enough memory blocks to be set online/offline. 4 update the memory block list info, and let guest agent to set memory blocks online/offline. Note that because we hotplug memory logically by online/offline MEMORY BLOCKS, and each memory block has a size much bigger than KiB, there's a deviation with the range of (0, block_size). block_size may be 128MB or 256MB or etc., it differs on different OSes. Zhang Bo (8): lifecycle: add flag VIR_DOMAIN_MEM_GUEST for viDomainSetMemoryFlags qemu: agent: define structure of qemuAgentMemblockInfo qemu: agent: implement qemuAgentGetMemblocks qemu: agent: implement qemuAgentGetMemblockGeneralInfo qemu: agent: implement qemuAgentUpdateMemblocks qemu: agent: implement function qemuAgetSetMemblocks qemu: memory: logically hotplug memory with guest agent virsh: support memory hotplug with guest agent in virsh include/libvirt/libvirt-domain.h | 1 + src/libvirt-domain.c | 7 + src/qemu/qemu_agent.c| 307 +++ src/qemu/qemu_agent.h| 22 +++ src/qemu/qemu_driver.c | 46 +- tools/virsh-domain.c | 10 +- tools/virsh.pod | 7 +- 7 files changed, 396 insertions(+), 4 deletions(-) -- 1.7.12.4 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 2/8] qemu: agent: define structure of qemuAgentMemblockInfo
add the definition of qemuAgentMemblockInfo, according to the json format:
{ 'struct': 'GuestMemoryBlock',
'data': {'phys-index': 'uint64',
'online': 'bool',
'*can-offline': 'bool'} }
Signed-off-by: Zhang Bo oscar.zhan...@huawei.com
Signed-off-by: Li Bin binlibin...@huawei.com
---
src/qemu/qemu_agent.h | 9 +
1 file changed, 9 insertions(+)
diff --git a/src/qemu/qemu_agent.h b/src/qemu/qemu_agent.h
index 7cbf8eb..425ee87 100644
--- a/src/qemu/qemu_agent.h
+++ b/src/qemu/qemu_agent.h
@@ -103,6 +103,15 @@ int qemuAgentUpdateCPUInfo(unsigned int nvcpus,
qemuAgentCPUInfoPtr cpuinfo,
int ncpuinfo);
+typedef struct _qemuAgentMemblockInfo qemuAgentMemblockInfo;
+typedef qemuAgentMemblockInfo *qemuAgentMemblockInfoPtr;
+struct _qemuAgentMemblockInfo {
+unsigned long long id; /* arbitrary guest-specific unique identifier of
the MEMORY BLOCK*/
+bool online;/* true if the MEMORY BLOCK is enabled in the guest*/
+bool offlinable;/* true if the MEMORY BLOCK can be offlined */
+};
+
+
int qemuAgentGetTime(qemuAgentPtr mon,
long long *seconds,
unsigned int *nseconds);
--
1.7.12.4
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 7/8] qemu: memory: logically hotplug memory with guest agent
hotplug memory with guest agent. It
1 get memory block list, each member has 'phy-index', 'online' and
'can-offline' parameters
2 get memory block size, normally 128MB or 256MB for most OSes
3 convert the target memory size to memory block number, and see if there's
enough memory
blocks to be set online/offline.
4 update the memory block list info, and let guest agent to set memory blocks
online/offline.
note: because we hotplug memory logically by online/offline MEMORY BLOCKS, and
each memory block has
a size much bigger than KiB, there's a deviation with the range of (0,
block_size).
Signed-off-by: Zhang Bo oscar.zhan...@huawei.com
Signed-off-by: Li Bin binlibin...@huawei.com
---
src/qemu/qemu_driver.c | 42 +-
1 file changed, 41 insertions(+), 1 deletion(-)
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index 580cd60..2a20bef 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -2307,6 +2307,10 @@ static int qemuDomainSetMemoryFlags(virDomainPtr dom,
unsigned long newmem,
virDomainDefPtr persistentDef;
int ret = -1, r;
virQEMUDriverConfigPtr cfg = NULL;
+qemuAgentMemblockInfoPtr memblocks = NULL;
+int nblocks = 0;
+qemuAgentMemblockGeneralInfoPtr meminfo = NULL;
+unsigned long long newmem_MB = newmem 10;
virCheckFlags(VIR_DOMAIN_AFFECT_LIVE |
VIR_DOMAIN_AFFECT_CONFIG |
@@ -2368,6 +2372,41 @@ static int qemuDomainSetMemoryFlags(virDomainPtr dom,
unsigned long newmem,
/* resize the current memory */
unsigned long oldmax = 0;
+priv = vm-privateData;
+
+if (flags VIR_DOMAIN_MEM_GUEST) {
+if (!qemuDomainAgentAvailable(vm, true))
+goto endjob;
+
+if (VIR_ALLOC(meminfo)) {
+virReportOOMError();
+goto endjob;
+}
+
+qemuDomainObjEnterAgent(vm);
+nblocks = qemuAgentGetMemblocks(priv-agent, memblocks);
+qemuDomainObjExitAgent(vm);
+
+if (nblocks 0)
+goto endjob;
+
+qemuDomainObjEnterAgent(vm);
+ret = qemuAgentGetMemblockGeneralInfo(priv-agent, meminfo);
+qemuDomainObjExitAgent(vm);
+
+if (ret 0)
+goto endjob;
+
+if (qemuAgentUpdateMemblocks(newmem_MB, memblocks, nblocks,
meminfo-blockSize))
+goto endjob;
+
+qemuDomainObjEnterAgent(vm);
+ret = qemuAgentSetMemblocks(priv-agent, memblocks, nblocks);
+qemuDomainObjExitAgent(vm);
+
+goto endjob;
+}
+
if (def)
oldmax = virDomainDefGetMemoryActual(def);
if (persistentDef) {
@@ -2382,7 +2421,6 @@ static int qemuDomainSetMemoryFlags(virDomainPtr dom,
unsigned long newmem,
}
if (def) {
-priv = vm-privateData;
qemuDomainObjEnterMonitor(driver, vm);
r = qemuMonitorSetBalloon(priv-mon, newmem);
if (qemuDomainObjExitMonitor(driver, vm) 0)
@@ -2415,6 +2453,8 @@ static int qemuDomainSetMemoryFlags(virDomainPtr dom,
unsigned long newmem,
cleanup:
virDomainObjEndAPI(vm);
virObjectUnref(cfg);
+VIR_FREE(meminfo);
+VIR_FREE(memblocks);
return ret;
}
--
1.7.12.4
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 6/8] qemu: agent: implement function qemuAgetSetMemblocks
qemuAgetSetMemblocks() is implemented, according to the qga command:
'guest-set-memory-blocks'.
It asks the guest agent to set memory blocks online/offline according to
the updated MemblockInfo. If all the blocks were setted successfully, the
function returns with success, otherwise, fails.
Signed-off-by: Zhang Bo oscar.zhan...@huawei.com
Signed-off-by: Li Bin binlibin...@huawei.com
---
src/qemu/qemu_agent.c | 117 ++
src/qemu/qemu_agent.h | 1 +
2 files changed, 118 insertions(+)
diff --git a/src/qemu/qemu_agent.c b/src/qemu/qemu_agent.c
index 2c3a5ba..1945fae 100644
--- a/src/qemu/qemu_agent.c
+++ b/src/qemu/qemu_agent.c
@@ -1846,6 +1846,123 @@ qemuAgentUpdateMemblocks(unsigned long long memory,
}
int
+qemuAgentSetMemblocks(qemuAgentPtr mon,
+ qemuAgentMemblockInfoPtr info,
+ int nblocks)
+{
+int ret = -1;
+virJSONValuePtr cmd = NULL;
+virJSONValuePtr reply = NULL;
+virJSONValuePtr memblocks = NULL;
+virJSONValuePtr block = NULL;
+virJSONValuePtr data = NULL;
+int size = -1;
+size_t i;
+
+/* create the key data array */
+if (!(memblocks = virJSONValueNewArray()))
+goto cleanup;
+
+for (i = 0; i nblocks; i++) {
+qemuAgentMemblockInfoPtr in = info[i];
+
+/* create single memory block object */
+if (!(block = virJSONValueNewObject()))
+goto cleanup;
+
+if (virJSONValueObjectAppendNumberInt(block, phys-index, in-id) 0)
+goto cleanup;
+
+if (virJSONValueObjectAppendBoolean(block, online, in-online) 0)
+goto cleanup;
+
+if (virJSONValueArrayAppend(memblocks, block) 0)
+goto cleanup;
+
+block = NULL;
+}
+
+if (!(cmd = qemuAgentMakeCommand(guest-set-memory-blocks,
+ a:mem-blks, memblocks,
+ NULL)))
+goto cleanup;
+
+memblocks = NULL;
+
+if (qemuAgentCommand(mon, cmd, reply, true,
+ VIR_DOMAIN_QEMU_AGENT_COMMAND_BLOCK) 0)
+goto cleanup;
+
+if (!(data = virJSONValueObjectGet(reply, return))) {
+virReportError(VIR_ERR_INTERNAL_ERROR, %s,
+ _(guest-set-memory-blocks reply was missing return
data));
+goto cleanup;
+}
+
+if (data-type != VIR_JSON_TYPE_ARRAY) {
+virReportError(VIR_ERR_INTERNAL_ERROR, %s,
+ _(guest-set-memory-blocks returned information was not
+ an array));
+goto cleanup;
+}
+
+if ((size = virJSONValueArraySize(data)) 0) {
+virReportError(VIR_ERR_INTERNAL_ERROR, %s,
+ _(qemu agent didn't return an array of results));
+goto cleanup;
+}
+
+for (i = 0; i size; i++) {
+virJSONValuePtr tmp_res = virJSONValueArrayGet(data, i);
+unsigned long long id = 0;
+const char *response = NULL;
+int error_code = 0;
+
+if (!tmp_res) {
+virReportError(VIR_ERR_INTERNAL_ERROR, %s,
+ _(qemu agent reply missing result entry in
array));
+goto cleanup;
+}
+
+if (virJSONValueObjectGetNumberUlong(tmp_res, phys-index, id) 0) {
+virReportError(VIR_ERR_INTERNAL_ERROR, %s,
+ _(qemu agent didn't provide 'phys-index'
correctly));
+goto cleanup;
+}
+
+if (!(response = virJSONValueObjectGetString(tmp_res, response))) {
+virReportError(VIR_ERR_INTERNAL_ERROR,
+ _(qemu agent didn't provide 'response'
+ field for memory block %llu), id);
+goto cleanup;
+}
+
+if (STRNEQ(response, success)) {
+virReportError(VIR_ERR_INTERNAL_ERROR,
+ _(qemu agent failed to set memory block %llu:
%s), id, response);
+if (virJSONValueObjectGetNumberInt(tmp_res, error-code,
error_code) 0) {
+virReportError(VIR_ERR_INTERNAL_ERROR, %s,
+ _(qemu agent didn't provide 'error-code' in
response));
+goto cleanup;
+}
+
+virReportError(VIR_ERR_INTERNAL_ERROR, _(errno-code is %d),
error_code);
+goto cleanup;
+ }
+}
+
+ret = 0;
+
+ cleanup:
+virJSONValueFree(cmd);
+virJSONValueFree(reply);
+virJSONValueFree(block);
+virJSONValueFree(memblocks);
+return ret;
+}
+
+
+int
qemuAgentGetTime(qemuAgentPtr mon,
long long *seconds,
unsigned int *nseconds)
diff --git a/src/qemu/qemu_agent.h b/src/qemu/qemu_agent.h
index 3ba6deb..9707510 100644
--- a/src/qemu/qemu_agent.h
+++ b/src/qemu/qemu_agent.h
@@ -123,6 +123,7 @@ int qemuAgentUpdateMemblocks(unsigned long long memory,
qemuAgentMemblockInfoPtr info
[libvirt] [PATCH 3/8] qemu: agent: implement qemuAgentGetMemblocks
implement function qemuAgentGetMemblocks().
behaviour example:
input: '{execute:guest-get-memory-blocks}'
output:
{
return: [
{
can-offline: false,
online: true,
phys-index: 0
},
{
can-offline: false,
online: true,
phys-index: 1
},
..
]
}
please refer to
http://git.qemu.org/?p=qemu.git;a=log;h=0dd38a03f5e1498aabf7d053a9fab792a5eeec5c
for more information.
Signed-off-by: Zhang Bo oscar.zhan...@huawei.com
Signed-off-by: Li Bin binlibin...@huawei.com
---
src/qemu/qemu_agent.c | 73 +++
src/qemu/qemu_agent.h | 1 +
2 files changed, 74 insertions(+)
diff --git a/src/qemu/qemu_agent.c b/src/qemu/qemu_agent.c
index 043695b..95daf7a 100644
--- a/src/qemu/qemu_agent.c
+++ b/src/qemu/qemu_agent.c
@@ -1654,6 +1654,79 @@ qemuAgentUpdateCPUInfo(unsigned int nvcpus,
return 0;
}
+int
+qemuAgentGetMemblocks(qemuAgentPtr mon,
+ qemuAgentMemblockInfoPtr *info)
+{
+int ret = -1;
+size_t i;
+virJSONValuePtr cmd = NULL;
+virJSONValuePtr reply = NULL;
+virJSONValuePtr data = NULL;
+int ndata;
+
+if (!(cmd = qemuAgentMakeCommand(guest-get-memory-blocks, NULL)))
+return -1;
+
+if (qemuAgentCommand(mon, cmd, reply, true,
+ VIR_DOMAIN_QEMU_AGENT_COMMAND_BLOCK) 0)
+goto cleanup;
+
+if (!(data = virJSONValueObjectGet(reply, return))) {
+virReportError(VIR_ERR_INTERNAL_ERROR, %s,
+ _(guest-get-memory-blocks reply was missing return
data));
+goto cleanup;
+}
+
+if (data-type != VIR_JSON_TYPE_ARRAY) {
+virReportError(VIR_ERR_INTERNAL_ERROR, %s,
+ _(guest-get-memory-blocks return information was not
an array));
+goto cleanup;
+}
+
+ndata = virJSONValueArraySize(data);
+
+if (VIR_ALLOC_N(*info, ndata) 0)
+goto cleanup;
+
+for (i = 0; i ndata; i++) {
+virJSONValuePtr entry = virJSONValueArrayGet(data, i);
+qemuAgentMemblockInfoPtr in = *info + i;
+
+if (!entry) {
+virReportError(VIR_ERR_INTERNAL_ERROR, %s,
+ _(array element missing in guest-get-memory-blocks
return
+ value));
+goto cleanup;
+}
+
+if (virJSONValueObjectGetNumberUint(entry, phys-index, in-id) 0)
{
+virReportError(VIR_ERR_INTERNAL_ERROR, %s,
+ _('phys-index' missing in reply of
guest-get-memory-blocks));
+goto cleanup;
+}
+
+if (virJSONValueObjectGetBoolean(entry, online, in-online) 0) {
+virReportError(VIR_ERR_INTERNAL_ERROR, %s,
+ _('online' missing in reply of
guest-get-memory-blocks));
+goto cleanup;
+}
+
+if (virJSONValueObjectGetBoolean(entry, can-offline,
+ in-offlinable) 0) {
+virReportError(VIR_ERR_INTERNAL_ERROR, %s,
+ _('can-offline' missing in reply of
guest-get-memory-blocks));
+goto cleanup;
+}
+}
+
+ret = ndata;
+
+ cleanup:
+virJSONValueFree(cmd);
+virJSONValueFree(reply);
+return ret;
+}
int
qemuAgentGetTime(qemuAgentPtr mon,
diff --git a/src/qemu/qemu_agent.h b/src/qemu/qemu_agent.h
index 425ee87..61ba038 100644
--- a/src/qemu/qemu_agent.h
+++ b/src/qemu/qemu_agent.h
@@ -111,6 +111,7 @@ struct _qemuAgentMemblockInfo {
bool offlinable;/* true if the MEMORY BLOCK can be offlined */
};
+int qemuAgentGetMemblocks(qemuAgentPtr mon, qemuAgentMemblockInfoPtr *info);
int qemuAgentGetTime(qemuAgentPtr mon,
long long *seconds,
--
1.7.12.4
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 5/8] qemu: agent: implement qemuAgentUpdateMemblocks
function qemuAgentUpdateMemblocks() checks whether it needs to plug/unplug
memory blocks to reach the target memory. it's similar to
qemuAgentUpdateCPUInfo().
Signed-off-by: Zhang Bo oscar.zhan...@huawei.com
Signed-off-by: Li Bin binlibin...@huawei.com
---
src/qemu/qemu_agent.c | 69 +++
src/qemu/qemu_agent.h | 4 +++
2 files changed, 73 insertions(+)
diff --git a/src/qemu/qemu_agent.c b/src/qemu/qemu_agent.c
index 3481354..2c3a5ba 100644
--- a/src/qemu/qemu_agent.c
+++ b/src/qemu/qemu_agent.c
@@ -1775,6 +1775,75 @@ qemuAgentGetMemblockGeneralInfo(qemuAgentPtr mon,
return ret;
}
+int
+qemuAgentUpdateMemblocks(unsigned long long memory,
+ qemuAgentMemblockInfoPtr info,
+ int nblock,
+ unsigned long long blocksize)
+{
+size_t i;
+int nonline = 0;
+int nofflinable = 0;
+unsigned long long ntarget = 0;
+
+if (memory % blocksize) {
+ntarget = (int)((memory / blocksize) + 1);
+}else {
+ntarget = (int)(memory / blocksize);
+}
+
+/* count the active and offlinable memory blocks */
+for (i = 0; i nblock; i++) {
+if (info[i].online)
+nonline++;
+
+if (info[i].offlinable info[i].online)
+nofflinable++;
+
+/* This shouldn't happen, but we can't trust the guest agent */
+if (!info[i].online !info[i].offlinable) {
+virReportError(VIR_ERR_INTERNAL_ERROR, %s,
+ _(Invalid data provided by guest agent));
+return -1;
+}
+}
+
+/* the guest agent reported less memory than requested */
+if (ntarget nblock) {
+virReportError(VIR_ERR_INTERNAL_ERROR, %s,
+ _(guest agent reports less memory than requested));
+return -1;
+}
+
+/* not enough offlinable memory blocks to support the request */
+if (ntarget (nonline - nofflinable)) {
+virReportError(VIR_ERR_INVALID_ARG, %s,
+ _(Cannot offline enough memory blocks));
+return -1;
+}
+
+for (i = 0; i nblock; i++) {
+if (ntarget nonline) {
+/* unplug */
+if (info[i].offlinable info[i].online) {
+info[i].online = false;
+nonline--;
+}
+} else if (ntarget nonline) {
+/* plug */
+if (!info[i].online) {
+info[i].online = true;
+nonline++;
+}
+} else {
+/* done */
+break;
+}
+}
+
+return 0;
+
+}
int
qemuAgentGetTime(qemuAgentPtr mon,
diff --git a/src/qemu/qemu_agent.h b/src/qemu/qemu_agent.h
index 9a9b859..3ba6deb 100644
--- a/src/qemu/qemu_agent.h
+++ b/src/qemu/qemu_agent.h
@@ -119,6 +119,10 @@ struct _qemuAgentMemblockGeneralInfo {
int qemuAgentGetMemblocks(qemuAgentPtr mon, qemuAgentMemblockInfoPtr *info);
int qemuAgentGetMemblockGeneralInfo(qemuAgentPtr mon,
qemuAgentMemblockGeneralInfoPtr info);
+int qemuAgentUpdateMemblocks(unsigned long long memory,
+ qemuAgentMemblockInfoPtr info,
+ int nblock,
+ unsigned long long blocksize);
int qemuAgentGetTime(qemuAgentPtr mon,
long long *seconds,
--
1.7.12.4
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 1/8] lifecycle: add flag VIR_DOMAIN_MEM_GUEST for viDomainSetMemoryFlags
just add the flag and description for function virDomainSetMemoryFlags().
Signed-off-by: Zhang Bo oscar.zhan...@huawei.com
Signed-off-by: Li Bin binlibin...@huawei.com
---
include/libvirt/libvirt-domain.h | 1 +
src/libvirt-domain.c | 4
src/qemu/qemu_driver.c | 3 ++-
3 files changed, 7 insertions(+), 1 deletion(-)
diff --git a/include/libvirt/libvirt-domain.h b/include/libvirt/libvirt-domain.h
index d851225..103266a 100644
--- a/include/libvirt/libvirt-domain.h
+++ b/include/libvirt/libvirt-domain.h
@@ -1163,6 +1163,7 @@ typedef enum {
/* Additionally, these flags may be bitwise-OR'd in. */
VIR_DOMAIN_MEM_MAXIMUM = (1 2), /* affect Max rather than current */
+VIR_DOMAIN_MEM_GUEST = (1 3), /* logically change memory size in the
guest */
} virDomainMemoryModFlags;
diff --git a/src/libvirt-domain.c b/src/libvirt-domain.c
index 7e6d749..155fb92 100644
--- a/src/libvirt-domain.c
+++ b/src/libvirt-domain.c
@@ -1945,6 +1945,10 @@ virDomainSetMemory(virDomainPtr domain, unsigned long
memory)
* on whether just live or both live and persistent state is changed.
* If VIR_DOMAIN_MEM_MAXIMUM is set, the change affects domain's maximum memory
* size rather than current memory size.
+ * If VIR_DOMAIN_MEM_GUEST is set, it changes the domain's memory size inside
+ * the guest instead of the hypervisor. This flag can only be used with live
guests.
+ * The usage of this flag may require a guest agent configured.
+ *
* Not all hypervisors can support all flag combinations.
*
* Returns 0 in case of success, -1 in case of failure.
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index 34e5581..580cd60 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -2310,7 +2310,8 @@ static int qemuDomainSetMemoryFlags(virDomainPtr dom,
unsigned long newmem,
virCheckFlags(VIR_DOMAIN_AFFECT_LIVE |
VIR_DOMAIN_AFFECT_CONFIG |
- VIR_DOMAIN_MEM_MAXIMUM, -1);
+ VIR_DOMAIN_MEM_MAXIMUM |
+ VIR_DOMAIN_MEM_GUEST, -1);
if (!(vm = qemuDomObjFromDomain(dom)))
goto cleanup;
--
1.7.12.4
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 4/8] qemu: agent: implement qemuAgentGetMemblockGeneralInfo
qemuAgentGetMemblockGeneralInfo() is implememted, according to the qga command
'guest-get-memory-block-info'.
the difference between this command and 'guest-get-memory-blocks' is that the
latter one gets a list of infos for each memory block, and this command just
returns
general attributes for the guest memory blocks.
Signed-off-by: Zhang Bo oscar.zhan...@huawei.com
Signed-off-by: Li Bin binlibin...@huawei.com
---
src/qemu/qemu_agent.c | 50 +-
src/qemu/qemu_agent.h | 7 +++
2 files changed, 56 insertions(+), 1 deletion(-)
diff --git a/src/qemu/qemu_agent.c b/src/qemu/qemu_agent.c
index 95daf7a..3481354 100644
--- a/src/qemu/qemu_agent.c
+++ b/src/qemu/qemu_agent.c
@@ -1700,7 +1700,7 @@ qemuAgentGetMemblocks(qemuAgentPtr mon,
goto cleanup;
}
-if (virJSONValueObjectGetNumberUint(entry, phys-index, in-id) 0)
{
+if (virJSONValueObjectGetNumberUlong(entry, phys-index, in-id)
0) {
virReportError(VIR_ERR_INTERNAL_ERROR, %s,
_('phys-index' missing in reply of
guest-get-memory-blocks));
goto cleanup;
@@ -1729,6 +1729,54 @@ qemuAgentGetMemblocks(qemuAgentPtr mon,
}
int
+qemuAgentGetMemblockGeneralInfo(qemuAgentPtr mon,
+qemuAgentMemblockGeneralInfoPtr info)
+{
+int ret = -1;
+unsigned long long json_size = 0;
+virJSONValuePtr cmd = NULL;
+virJSONValuePtr reply = NULL;
+virJSONValuePtr data = NULL;
+
+if (!info) {
+VIR_ERROR(_(NULL info));
+return ret;
+}
+
+cmd = qemuAgentMakeCommand(guest-get-memory-block-info,
+ NULL);
+if (!cmd)
+return ret;
+
+if (qemuAgentCommand(mon, cmd, reply, true,
+ VIR_DOMAIN_QEMU_AGENT_COMMAND_BLOCK) 0)
+goto cleanup;
+
+if (!(data = virJSONValueObjectGet(reply, return))) {
+virReportError(VIR_ERR_INTERNAL_ERROR, %s,
+ _(guest-get-memory-block-info reply was missing return
data));
+goto cleanup;
+}
+
+if (virJSONValueObjectGetNumberUlong(data, size, json_size) 0) {
+virReportError(VIR_ERR_INTERNAL_ERROR, %s,
+ _('size' missing in reply of
guest-get-memory-block-info));
+goto cleanup;
+}
+
+/* guest agent returns the size in Bytes,
+ * we change it into MB here */
+info-blockSize = json_size 20;
+ret = 0;
+
+ cleanup:
+virJSONValueFree(cmd);
+virJSONValueFree(reply);
+return ret;
+}
+
+
+int
qemuAgentGetTime(qemuAgentPtr mon,
long long *seconds,
unsigned int *nseconds)
diff --git a/src/qemu/qemu_agent.h b/src/qemu/qemu_agent.h
index 61ba038..9a9b859 100644
--- a/src/qemu/qemu_agent.h
+++ b/src/qemu/qemu_agent.h
@@ -111,7 +111,14 @@ struct _qemuAgentMemblockInfo {
bool offlinable;/* true if the MEMORY BLOCK can be offlined */
};
+typedef struct _qemuAgentMemblockGeneralInfo qemuAgentMemblockGeneralInfo;
+typedef qemuAgentMemblockGeneralInfo *qemuAgentMemblockGeneralInfoPtr;
+struct _qemuAgentMemblockGeneralInfo {
+unsigned long long blockSize;
+};
+
int qemuAgentGetMemblocks(qemuAgentPtr mon, qemuAgentMemblockInfoPtr *info);
+int qemuAgentGetMemblockGeneralInfo(qemuAgentPtr mon,
qemuAgentMemblockGeneralInfoPtr info);
int qemuAgentGetTime(qemuAgentPtr mon,
long long *seconds,
--
1.7.12.4
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 8/8] virsh: support memory hotplug with guest agent in virsh
support memory hotplug with the arg --guest in virsh command 'setmem'.
fix a little bug in qemu_driver.c at the meanwhile.
Signed-off-by: Zhang Bo oscar.zhan...@huawei.com
Signed-off-by: Li Bin binlibin...@huawei.com
---
src/libvirt-domain.c | 5 -
src/qemu/qemu_driver.c | 3 ++-
tools/virsh-domain.c | 10 +-
tools/virsh.pod| 7 ++-
4 files changed, 21 insertions(+), 4 deletions(-)
diff --git a/src/libvirt-domain.c b/src/libvirt-domain.c
index 155fb92..a1250b6 100644
--- a/src/libvirt-domain.c
+++ b/src/libvirt-domain.c
@@ -1947,7 +1947,10 @@ virDomainSetMemory(virDomainPtr domain, unsigned long
memory)
* size rather than current memory size.
* If VIR_DOMAIN_MEM_GUEST is set, it changes the domain's memory size inside
* the guest instead of the hypervisor. This flag can only be used with live
guests.
- * The usage of this flag may require a guest agent configured.
+ * The usage of this flag may require a guest agent configured. Note that
because we
+ * hotplug memory logically by online/offline MEMORY BLOCKS, and each memory
block has
+ * a size much bigger than KiB, there's a deviation with the range of (0,
block_size).
+ * block_size may be 128MB or 256MB or etc., it differs on different OSes.
*
* Not all hypervisors can support all flag combinations.
*
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index 2a20bef..e96465c 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -2397,7 +2397,8 @@ static int qemuDomainSetMemoryFlags(virDomainPtr dom,
unsigned long newmem,
if (ret 0)
goto endjob;
-if (qemuAgentUpdateMemblocks(newmem_MB, memblocks, nblocks,
meminfo-blockSize))
+ret = qemuAgentUpdateMemblocks(newmem_MB, memblocks, nblocks,
meminfo-blockSize);
+if (ret 0)
goto endjob;
qemuDomainObjEnterAgent(vm);
diff --git a/tools/virsh-domain.c b/tools/virsh-domain.c
index a25b7ba..ddb1cf9 100644
--- a/tools/virsh-domain.c
+++ b/tools/virsh-domain.c
@@ -8333,6 +8333,10 @@ static const vshCmdOptDef opts_setmem[] = {
.type = VSH_OT_BOOL,
.help = N_(affect current domain)
},
+{.name = guest,
+ .type = VSH_OT_BOOL,
+ .help = N_(use guest agent based hotplug, by enabling/disabling memory
blocks)
+},
{.name = NULL}
};
@@ -8347,17 +8351,21 @@ cmdSetmem(vshControl *ctl, const vshCmd *cmd)
bool config = vshCommandOptBool(cmd, config);
bool live = vshCommandOptBool(cmd, live);
bool current = vshCommandOptBool(cmd, current);
+bool guest = vshCommandOptBool(cmd, guest);
unsigned int flags = VIR_DOMAIN_AFFECT_CURRENT;
VSH_EXCLUSIVE_OPTIONS_VAR(current, live);
VSH_EXCLUSIVE_OPTIONS_VAR(current, config);
+VSH_EXCLUSIVE_OPTIONS_VAR(guest, config);
if (config)
flags |= VIR_DOMAIN_AFFECT_CONFIG;
if (live)
flags |= VIR_DOMAIN_AFFECT_LIVE;
+if (guest)
+flags |= VIR_DOMAIN_MEM_GUEST;
/* none of the options were specified */
-if (!current !live !config)
+if (!current flags == 0)
flags = -1;
if (!(dom = vshCommandOptDomain(ctl, cmd, NULL)))
diff --git a/tools/virsh.pod b/tools/virsh.pod
index 4e3f82a..534cc5e 100644
--- a/tools/virsh.pod
+++ b/tools/virsh.pod
@@ -1988,7 +1988,7 @@ BExamples
virsh send-process-signal myguest 1 SIG_HUP
=item Bsetmem Idomain Bsize [[I--config] [I--live] |
-[I--current]]
+[I--current]] [I--guest]
Change the memory allocation for a guest domain.
If I--live is specified, perform a memory balloon of a running guest.
@@ -1997,6 +1997,11 @@ If I--current is specified, affect the current guest
state.
Both I--live and I--config flags may be given, but I--current is
exclusive. If no flag is specified, behavior is different depending
on hypervisor.
+If I--guest is specified, it use guest agent based hotplug, by
+enabling/disabling memory blocks. Note that because we hotplug memory logically
+by online/offline MEMORY BLOCKS, and each memory block has a size much bigger
+than KiB, there's a deviation with the range of (0, block_size). block_size
+may be 128MB or 256MB or etc., it differs on different OSes.
Isize is a scaled integer (see BNOTES above); it defaults to kibibytes
(blocks of 1024 bytes) unless you provide a suffix (and the older option
--
1.7.12.4
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 0/8] logically memory hotplug via guest agent
On 2015/6/9 20:47, Daniel P. Berrange wrote: On Tue, Jun 09, 2015 at 02:12:39PM +0200, Peter Krempa wrote: On Tue, Jun 09, 2015 at 13:05:35 +0100, Daniel Berrange wrote: On Tue, Jun 09, 2015 at 02:03:13PM +0200, Peter Krempa wrote: On Tue, Jun 09, 2015 at 12:46:27 +0100, Daniel Berrange wrote: On Tue, Jun 09, 2015 at 01:22:49PM +0200, Peter Krempa wrote: On Tue, Jun 09, 2015 at 11:05:16 +0100, Daniel Berrange wrote: On Tue, Jun 09, 2015 at 05:33:24PM +0800, Zhang Bo wrote: ... 2) The guest OS sometimes needs to enable the memory region after ACPI memory hotplug. The GA would be able to online such memory. For this option we don't need to go through a different API though since it can be compounded using a flag. So, are you saying that we should not be adding this to the virDomainSetMemory API as done in this series, and we should instead be able to request automatic enabling/disabling of the regions when we do the original DIMM hotplug ? Well, that's the only place where using the memory region GA apis would make sense for libvirt. Whether we should do it is not that clear. Windows does online the regions automatically and I was told that some linux distros do it via udev rules. What do we do in the case of hotunplug currently ? Are we expectig the guest admin to have manually offlined the regions before doing hotunplug on the host ? You don't need to offline them prior to unplug. The guest OS handles that automatically when it receives the request. Hmm, so if the guest can offline and online DIMMS automatically on hotplug/unplug, then I'm puzzelled what value this patch series really adds. Regards, Daniel Thank you for your reply. Before this patch, we needed to manually online memory blocks inside the guest, after dimm memory hotplug for most *nix OSes. (Windows guests automatically get their memory blocks online after hotplugging) That is to say, we need to LOGICALLY hotplug memory after PHYSICAL hotplug. This patch did the LOGICAL part. With this patch, we don't need to get into the guest to manually online them anymore, which is even impossible for most host administrators. -- Oscar oscar.zhan...@huawei.com -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH] qemu: update netdevs of the same mac addrs correctly
If a guest has multiple network devices with the same MAC address,
when we online update the second device, libvirtd always updates
the first one.
commit def31e4c forgot to fix the online updating scenario. We need to
use virDomainNetFindIdx() to find the correct network device.
Signed-off-by: Zhou Yimin zhouyi...@huawei.com
Signed-off-by: Zhang Bo oscar.zhan...@huawei.com
---
src/qemu/qemu_hotplug.c | 23 +++
1 file changed, 7 insertions(+), 16 deletions(-)
diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c
index 94ebe35..d455bd6 100644
--- a/src/qemu/qemu_hotplug.c
+++ b/src/qemu/qemu_hotplug.c
@@ -2052,20 +2052,6 @@ int qemuDomainAttachHostDevice(virConnectPtr conn,
return -1;
}
-static virDomainNetDefPtr *qemuDomainFindNet(virDomainObjPtr vm,
- virDomainNetDefPtr dev)
-{
-size_t i;
-
-for (i = 0; i vm-def-nnets; i++) {
-if (virMacAddrCmp(vm-def-nets[i]-mac, dev-mac) == 0)
-return vm-def-nets[i];
-}
-
-return NULL;
-}
-
-
static int
qemuDomainChangeNetBridge(virDomainObjPtr vm,
virDomainNetDefPtr olddev,
@@ -2195,7 +2181,7 @@ qemuDomainChangeNet(virQEMUDriverPtr driver,
virDomainDeviceDefPtr dev)
{
virDomainNetDefPtr newdev = dev-data.net;
-virDomainNetDefPtr *devslot = qemuDomainFindNet(vm, newdev);
+virDomainNetDefPtr *devslot = NULL;
virDomainNetDefPtr olddev;
int oldType, newType;
bool needReconnect = false;
@@ -2205,8 +2191,13 @@ qemuDomainChangeNet(virQEMUDriverPtr driver,
bool needReplaceDevDef = false;
bool needBandwidthSet = false;
int ret = -1;
+int changeidx = -1;
+
+if ((changeidx = virDomainNetFindIdx(vm-def, newdev)) 0)
+goto cleanup;
+devslot = vm-def-nets[changeidx];
-if (!devslot || !(olddev = *devslot)) {
+if (!(olddev = *devslot)) {
virReportError(VIR_ERR_OPERATION_FAILED, %s,
_(cannot find existing network device to modify));
goto cleanup;
--
1.7.12.4
--
Oscar
oscar.zhan...@huawei.com
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] Libvirt now using Zanata for translation
On 2015/5/27 17:00, Daniel P. Berrange wrote: On Wed, May 27, 2015 at 08:45:09AM +0800, zhang bo wrote: On 2015/3/6 21:20, Daniel P. Berrange wrote: As of current GIT master, libvirt is fully using Zanata for po file translation After some trouble with the zanata python client, I have now successfully refreshed pushed the current libvirt.pot file, and synchronized all the translation po files back down to GIT master. For ongoing refreshes of the libvirt.pot, it can be rebuild pushed to zanata and .po files resynchonized using # cd po # rm libvirt.pot # make libvirt.pot # zanata-cli push # zanata-cli pull Where 'zanata-cli' is the *JAVA* client, not the python client. I strongly recommend against use of the python client for the libvirt project as it doesn't handle the size of the libvirt.pot/.po files well resulting in unexplained data loss. Even the java client has problems pushing the .po files to the server due to poorly designed rate limiting in the zanata server. Fortunately this is not something we need do again, now we have imported the existing .po file. The java client has been troublefree wrt to pushing the .pot file and pulling .po files, which is all we need for ongoing work now. Currently myself Daniel Veillard are setup as admins of the libvirt project in Zanata we can add further people as required. Regards, Daniel Would you please add me as admin so that I could help to translate and review (in Chinese)? The libvirt translations are managed under the Fedora translations teams. So in fact you just need to request to join the Chinese Fedora team. If you go to this page: https://fedora.zanata.org/language/view/zh-CN There's a drop down menu 'v ...' which has a 'Request to join team' option in it. Once you've joined you'd be able to work on any Fedora related project, or just libvirt, as you so prefer. Regards, Daniel Thanks! waiting for the coordinator to accept my application. -- Oscar oscar.zhan...@huawei.com -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] Libvirt now using Zanata for translation
On 2015/3/6 21:20, Daniel P. Berrange wrote: As of current GIT master, libvirt is fully using Zanata for po file translation After some trouble with the zanata python client, I have now successfully refreshed pushed the current libvirt.pot file, and synchronized all the translation po files back down to GIT master. For ongoing refreshes of the libvirt.pot, it can be rebuild pushed to zanata and .po files resynchonized using # cd po # rm libvirt.pot # make libvirt.pot # zanata-cli push # zanata-cli pull Where 'zanata-cli' is the *JAVA* client, not the python client. I strongly recommend against use of the python client for the libvirt project as it doesn't handle the size of the libvirt.pot/.po files well resulting in unexplained data loss. Even the java client has problems pushing the .po files to the server due to poorly designed rate limiting in the zanata server. Fortunately this is not something we need do again, now we have imported the existing .po file. The java client has been troublefree wrt to pushing the .pot file and pulling .po files, which is all we need for ongoing work now. Currently myself Daniel Veillard are setup as admins of the libvirt project in Zanata we can add further people as required. Regards, Daniel Would you please add me as admin so that I could help to translate and review (in Chinese)? -- Oscar oscar.zhan...@huawei.com -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] po: fix translation error for caller in zh_CN.po
On 2015/5/26 10:43, Eric Blake wrote: On 05/25/2015 08:28 PM, zhang bo wrote: IME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit caller should be translated to 璋冪敤绋嬪簭 rather than 璋冨害绋嬪簭, which in fact refers to scheduler in English. --- po/zh_CN.po | 8 1 file changed, 4 insertions(+), 4 deletions(-) Thanks; but this is not the right place for translation bugs. See https://www.redhat.com/archives/libvir-list/2015-March/msg00302.html for instructions on submitting translations to zanata. -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list I registered an account on zanata.org, and found the website: https://fedora.zanata.org/app/#/libvirt/master/translate/libvirt/zh-CN/?id=2094492selected=true But when I finished translation, and tried to click 'save as translated', the words I just typed disappeared as if I never edited there before. Any more work needs to be done? BTW: I'm working on Windows with Google Chrome to visit that website, zanata-cli not installed. -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] po: fix translation error for caller in zh_CN.po
On 2015/5/26 10:48, Eric Blake wrote: On 05/25/2015 08:43 PM, Eric Blake wrote: On 05/25/2015 08:28 PM, zhang bo wrote: IME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit caller should be translated to 璋冪敤绋嬪簭 rather than 璋冨害绋嬪簭, which in fact refers to scheduler in English. --- po/zh_CN.po | 8 1 file changed, 4 insertions(+), 4 deletions(-) Thanks; but this is not the right place for translation bugs. See https://www.redhat.com/archives/libvir-list/2015-March/msg00302.html for instructions on submitting translations to zanata. And we should really make that more prominent in HACKING; patches welcome... thank you Eric, I'm looking into zanata now and patches will be pushed later. -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH] po: fix translation error for caller in zh_CN.po
IME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit caller should be translated to 璋冪敤绋嬪簭 rather than 璋冨害绋嬪簭, which in fact refers to scheduler in English. --- po/zh_CN.po | 8 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/po/zh_CN.po b/po/zh_CN.po index f5c7cf1..38cf6a9 100644 --- a/po/zh_CN.po +++ b/po/zh_CN.po @@ -20800,12 +20800,12 @@ msgstr 瀹夊叏 doi 瓒呰繃鏈€澶ч暱搴︼細%zu #: src/remote/remote_driver.c:2639 msgid caller ignores cookie or cookielen -msgstr 璋冨害绋嬪簭蹇界暐 cookie 鎴栬€?cookielen +msgstr 璋冪敤绋嬪簭蹇界暐 cookie 鎴栬€?cookielen #: src/remote/remote_driver.c:2648 src/remote/remote_driver.c:6106 #: src/remote/remote_driver.c:7136 msgid caller ignores uri_out -msgstr 璋冨害绋嬪簭蹇界暐 uri_out +msgstr 璋冪敤绋嬪簭蹇界暐 uri_out #: src/remote/remote_driver.c:2781 #, c-format @@ -20892,7 +20892,7 @@ msgstr 鏃?internalFlags 鏀寔 #: src/remote/remote_driver.c:7127 src/remote/remote_driver.c:7225 #: src/remote/remote_driver.c:7297 src/remote/remote_driver.c:7370 msgid caller ignores cookieout or cookieoutlen -msgstr 鍡茬敤绋嬪簭蹇界暐 cookieout 鎴栬€?cookieoutlen +msgstr 璋冪敤绋嬪簭蹇界暐 cookieout 鎴栬€?cookieoutlen #: src/remote/remote_driver.c:6386 #, c-format @@ -25090,7 +25090,7 @@ msgstr 鍦?Win32 骞冲彴涓儴鏀寔鎵ц鏂拌繘绋? #: src/util/vircommand.c:2224 msgid cannot mix caller fds with blocking execution -msgstr 鏃犳硶灏嗚皟搴︾▼搴?fd 涓庨樆姝㈡墽琛屾贩鍚? +msgstr 鏃犳硶灏嗚皟鐢ㄧ▼搴?fd 涓庨樆姝㈡墽琛屾贩鍚? #: src/util/vircommand.c:2230 msgid cannot mix string I/O with daemon -- 1.7.12.4 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] po: fix translation error for caller in zh_CN.po
I don't know why the codes in chinese become messy-code in this patch for me. I'm using thunderBird on *Windows*. The patch looked fine on my *Linux* machine. How does it appear at your side? If there's more work to do to make it look normal, please let me know. BTW: shall I change the headers of zh_CN.po at the meanwhile? eg. POT-Creation-Date, PO-Revision-Date, Last-Translator. On 2015/5/26 10:28, zhang bo wrote: IME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit caller should be translated to 璋冪敤绋嬪簭 rather than 璋冨害绋嬪簭, which in fact refers to scheduler in English. --- po/zh_CN.po | 8 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/po/zh_CN.po b/po/zh_CN.po index f5c7cf1..38cf6a9 100644 --- a/po/zh_CN.po +++ b/po/zh_CN.po @@ -20800,12 +20800,12 @@ msgstr 瀹夊叏 doi 瓒呰繃鏈€澶ч暱搴︼細%zu #: src/remote/remote_driver.c:2639 msgid caller ignores cookie or cookielen -msgstr 璋冨害绋嬪簭蹇界暐 cookie 鎴栬€?cookielen +msgstr 璋冪敤绋嬪簭蹇界暐 cookie 鎴栬€?cookielen #: src/remote/remote_driver.c:2648 src/remote/remote_driver.c:6106 #: src/remote/remote_driver.c:7136 msgid caller ignores uri_out -msgstr 璋冨害绋嬪簭蹇界暐 uri_out +msgstr 璋冪敤绋嬪簭蹇界暐 uri_out #: src/remote/remote_driver.c:2781 #, c-format @@ -20892,7 +20892,7 @@ msgstr 鏃?internalFlags 鏀寔 #: src/remote/remote_driver.c:7127 src/remote/remote_driver.c:7225 #: src/remote/remote_driver.c:7297 src/remote/remote_driver.c:7370 msgid caller ignores cookieout or cookieoutlen -msgstr 鍡茬敤绋嬪簭蹇界暐 cookieout 鎴栬€?cookieoutlen +msgstr 璋冪敤绋嬪簭蹇界暐 cookieout 鎴栬€?cookieoutlen #: src/remote/remote_driver.c:6386 #, c-format @@ -25090,7 +25090,7 @@ msgstr 鍦?Win32 骞冲彴涓儴鏀寔鎵ц鏂拌繘绋? #: src/util/vircommand.c:2224 msgid cannot mix caller fds with blocking execution -msgstr 鏃犳硶灏嗚皟搴︾▼搴?fd 涓庨樆姝㈡墽琛屾贩鍚? +msgstr 鏃犳硶灏嗚皟鐢ㄧ▼搴?fd 涓庨樆姝㈡墽琛屾贩鍚? #: src/util/vircommand.c:2230 msgid cannot mix string I/O with daemon -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH] util: make it more robust to calculate timeout value
When we change system clock to years ago, a certain CPU may use up 100% cputime.
The reason is that in function virEventPollCalculateTimeout(), we assign the
unsigned long long result to an INT variable,
*timeout = then - now; // timeout is INT, and then/now are long long
if (*timeout 0)
*timeout = 0;
there's a chance that variable @then minus variable @now may be a very large
number
that overflows INT value expression, then *timeout will be negative and be
assigned to 0.
Next the 'poll' in function virEventPollRunOnce() will get into an 'endless'
while loop there.
thus, the cpu that virEventPollRunOnce() thread runs on will go up to 100%.
Although as we discussed before in
https://www.redhat.com/archives/libvir-list/2015-May/msg00400.html
it should be prohibited to set-time while other applications are running, but
it does
seems to have no harm to make the codes more robust.
Signed-off-by: Wang Yufei james.wangyu...@huawei.com
Signed-off-by: Zhang Bo oscar.zhan...@huawei.com
---
src/util/vireventpoll.c | 5 +++--
1 file changed, 3 insertions(+), 2 deletions(-)
diff --git a/src/util/vireventpoll.c b/src/util/vireventpoll.c
index ffda206..5f5a149 100644
--- a/src/util/vireventpoll.c
+++ b/src/util/vireventpoll.c
@@ -357,9 +357,10 @@ static int virEventPollCalculateTimeout(int *timeout)
return -1;
EVENT_DEBUG(Schedule timeout then=%llu now=%llu, then, now);
-*timeout = then - now;
-if (*timeout 0)
+if (then now)
*timeout = 0;
+else
+*timeout = (then - now) 0x7FFF;
} else {
*timeout = -1;
}
--
1.7.12.4
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH] qemu: clear useless code
The variable 'now' is useless in qemuMigrationPrepareAny(), so I clear it.
Signed-off-by: Wang Yufei james.wangyu...@huawei.com
Signed-off-by: Zhang Bo oscar.zhan...@huawei.com
---
src/qemu/qemu_migration.c | 4
1 file changed, 4 deletions(-)
diff --git a/src/qemu/qemu_migration.c b/src/qemu/qemu_migration.c
index c1af704..4dcba7a 100644
--- a/src/qemu/qemu_migration.c
+++ b/src/qemu/qemu_migration.c
@@ -2913,7 +2913,6 @@ qemuMigrationPrepareAny(virQEMUDriverPtr driver,
int ret = -1;
int dataFD[2] = { -1, -1 };
qemuDomainObjPrivatePtr priv = NULL;
-unsigned long long now;
qemuMigrationCookiePtr mig = NULL;
bool tunnel = !!st;
char *xmlout = NULL;
@@ -2923,9 +2922,6 @@ qemuMigrationPrepareAny(virQEMUDriverPtr driver,
bool abort_on_error = !!(flags VIR_MIGRATE_ABORT_ON_ERROR);
bool taint_hook = false;
-if (virTimeMillisNow(now) 0)
-return -1;
-
virNWFilterReadLockFilterUpdates();
if (flags VIR_MIGRATE_OFFLINE) {
--
1.7.12.4
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] time: event poll may become un-triggerable after changing system clock.
event poll may become un-triggerable after changing system clock.
The steps to reproduce the problem:
1 run event-test
1 define and start a domain with name vm1.
2 destroy vm1
3 change system time to 1 hour before when timer.expiresAt has been set in
virEventPollUpdateTimeout
(and before virEventPollDispatchTimeouts()).
4 event-test will recive no message until 1 hour later.
The reasons for the problem is :
1 The value of timer.expiresAt is set by virTimeMillisNowRaw.
virTimeMillisNowRaw is effectable by settimeofday(),
bacause it uses CLOCK_REALTIME to get time.
2 If we change the system time to a time long before now, after that
timer.expiresAt has been set. timer.expiresAt
is not affected, while virEventPollDispatchTimeouts is.
Suppose it's now May 12th, and we set it to 10th, then the expiresAt is
12th, and the time virEventPollDispatchTimeouts
got is 10th.
if (eventLoop.timeouts[i].expiresAt = (now+20)) { // expiresAt will
not be less than now until 2 days later.
*Solution(not good enough)*:
1 change the clock mode in virTimeMillisNowRaw from REALTIME to MONOTONIC,
which would not be affected by
settimeofday().
2 add the time got from clock_gettime(*MONOTONIC*) with the system-start-time
from epoch, making it equal to the value got from REALTIME.
3 As that the timestamp of the log message should follow system time, so we
keep it to REALTIME as before.
However, there's still problems:
1 pthread_cond_wait() gets time with REALTIME mode. When we change system time,
pthread_cond_wait() may still be affected.
So, Is there any other better solution? thanks in advance.
oscar
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] time: event poll may become un-triggerable after changing system clock.
On 2015/5/12 16:56, Daniel P. Berrange wrote: On Tue, May 12, 2015 at 04:14:37PM +0800, zhang bo wrote: So, Is there any other better solution? thanks in advance. Simply don't change the system time by massive deltas. Libvirt is not going to be the only app to be affected. As you mention it is going to hit the pthread_cond_wait() call which will likely affect pretty much every single non-trivial process running on the system. I'd expect other apps have much the same problem with calculating poll sleeps too. If you need to massively change the system time this should be done at single user mode, or do a reboot. Once a system is running it should be kept synced with NTPD which will only ever change system time in very small increments and so once cause thsi problem. Regards, Daniel Thank you Daniel. Would you please explain more about what single user mode mean here? and does reboot refers to rebooting the os or rebooting the process libvirtd? I think it's the process, right? -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 0/2] remove 2 unused functions
From: YueWenyuan yueweny...@huawei.com remove unused functions virTimeFieldsNow() and virTimeFieldsNowRaw() YueWenyuan (2): remove unused function virTimeFieldsNow remove unused function virTimeFieldsNowRaw src/libvirt_private.syms | 2 -- src/util/virtime.c | 43 --- src/util/virtime.h | 4 3 files changed, 49 deletions(-) -- 1.7.12.4 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 2/2] remove unused function virTimeFieldsNowRaw
From: YueWenyuan yueweny...@huawei.com
remove unused function virTimeFieldsNowRaw() in src/libvirt_private.syms,
src/util/virtime.c and src/util/virtime.h
Signed-off-by: YueWenyuan yueweny...@huawei.com
Signed-off-by: Zhang Bo oscar.zhan...@huawei.com
---
src/libvirt_private.syms | 1 -
src/util/virtime.c | 22 --
src/util/virtime.h | 2 --
3 files changed, 25 deletions(-)
diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
index 43b041f..800fe95 100644
--- a/src/libvirt_private.syms
+++ b/src/libvirt_private.syms
@@ -2230,7 +2230,6 @@ virThreadPoolSendJob;
# util/virtime.h
-virTimeFieldsNowRaw;
virTimeFieldsThen;
virTimeLocalOffsetFromUTC;
virTimeMillisNow;
diff --git a/src/util/virtime.c b/src/util/virtime.c
index 7013caa..f127166 100644
--- a/src/util/virtime.c
+++ b/src/util/virtime.c
@@ -78,28 +78,6 @@ int virTimeMillisNowRaw(unsigned long long *now)
}
-/**
- * virTimeFieldsNowRaw:
- * @fields: filled with current time fields
- *
- * Retrieves the current time, in broken-down field format.
- * The time is always in UTC.
- *
- * Returns 0 on success, -1 on error with errno set
- */
-int virTimeFieldsNowRaw(struct tm *fields)
-{
-unsigned long long now;
-
-if (virTimeMillisNowRaw(now) 0)
-return -1;
-
-virTimeFieldsThen(now, fields);
-
-return 0;
-}
-
-
#define SECS_PER_HOUR (60 * 60)
#define SECS_PER_DAY(SECS_PER_HOUR * 24)
#define DIV(a, b) ((a) / (b) - ((a) % (b) 0))
diff --git a/src/util/virtime.h b/src/util/virtime.h
index 86dddc1..12c1303 100644
--- a/src/util/virtime.h
+++ b/src/util/virtime.h
@@ -44,8 +44,6 @@ void virTimeFieldsThen(unsigned long long when, struct tm
*fields)
* errno on failure */
int virTimeMillisNowRaw(unsigned long long *now)
ATTRIBUTE_NONNULL(1) ATTRIBUTE_RETURN_CHECK;
-int virTimeFieldsNowRaw(struct tm *fields)
-ATTRIBUTE_NONNULL(1) ATTRIBUTE_RETURN_CHECK;
int virTimeStringNowRaw(char *buf)
ATTRIBUTE_NONNULL(1) ATTRIBUTE_RETURN_CHECK;
int virTimeStringThenRaw(unsigned long long when, char *buf)
--
1.7.12.4
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 1/2] remove unused function virTimeFieldsNow
From: YueWenyuan yueweny...@huawei.com
remove unused function virTimeFieldsNow() in src/libvirt_private.syms,
src/util/virtime.c and src/util/virtime.h
Signed-off-by: YueWenyuan yueweny...@huawei.com
Signed-off-by: Zhang Bo oscar.zhan...@huawei.com
---
src/libvirt_private.syms | 1 -
src/util/virtime.c | 21 -
src/util/virtime.h | 2 --
3 files changed, 24 deletions(-)
diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
index 8c50ea2..43b041f 100644
--- a/src/libvirt_private.syms
+++ b/src/libvirt_private.syms
@@ -2230,7 +2230,6 @@ virThreadPoolSendJob;
# util/virtime.h
-virTimeFieldsNow;
virTimeFieldsNowRaw;
virTimeFieldsThen;
virTimeLocalOffsetFromUTC;
diff --git a/src/util/virtime.c b/src/util/virtime.c
index 9d365d5..7013caa 100644
--- a/src/util/virtime.c
+++ b/src/util/virtime.c
@@ -242,27 +242,6 @@ int virTimeMillisNow(unsigned long long *now)
/**
- * virTimeFieldsNowRaw:
- * @fields: filled with current time fields
- *
- * Retrieves the current time, in broken-down field format.
- * The time is always in UTC.
- *
- * Returns 0 on success, -1 on error with errno reported
- */
-int virTimeFieldsNow(struct tm *fields)
-{
-unsigned long long now;
-
-if (virTimeMillisNow(now) 0)
-return -1;
-
-virTimeFieldsThen(now, fields);
-return 0;
-}
-
-
-/**
* virTimeStringNow:
*
* Creates a string containing a formatted timestamp
diff --git a/src/util/virtime.h b/src/util/virtime.h
index 8ebad38..86dddc1 100644
--- a/src/util/virtime.h
+++ b/src/util/virtime.h
@@ -56,8 +56,6 @@ int virTimeStringThenRaw(unsigned long long when, char *buf)
*/
int virTimeMillisNow(unsigned long long *now)
ATTRIBUTE_NONNULL(1) ATTRIBUTE_RETURN_CHECK;
-int virTimeFieldsNow(struct tm *fields)
-ATTRIBUTE_NONNULL(1) ATTRIBUTE_RETURN_CHECK;
char *virTimeStringNow(void);
char *virTimeStringThen(unsigned long long when);
--
1.7.12.4
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 6/6] conf: fix memleak in virDomainHostdevDefClear
use virNetworkRouteDefFree() instead of VIR_FREE to free routes, otherwise the element 'family' would not be freed. Signed-off-by: Zhang Bo oscar.zhan...@huawei.com --- src/conf/domain_conf.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 8350fe7..f383104 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -1881,7 +1881,7 @@ void virDomainHostdevDefClear(virDomainHostdevDefPtr def) VIR_FREE(def-source.caps.u.net.ips[i]); VIR_FREE(def-source.caps.u.net.ips); for (i = 0; i def-source.caps.u.net.nroutes; i++) -VIR_FREE(def-source.caps.u.net.routes[i]); +virNetworkRouteDefFree(def-source.caps.u.net.routes[i]); VIR_FREE(def-source.caps.u.net.routes); break; } -- 1.7.12.4 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 4/6] qemu: fix memleak in virCapabilitiesDomainDataLookup
virBufferContentAndReset() doesn't free buf contents, we should use
virBufferFreeAndReset() to get buf freed.
Signed-off-by: Zhang Bo oscar.zhan...@huawei.com
---
src/conf/capabilities.c | 5 +++--
1 file changed, 3 insertions(+), 2 deletions(-)
diff --git a/src/conf/capabilities.c b/src/conf/capabilities.c
index 2c674a8..922741f 100644
--- a/src/conf/capabilities.c
+++ b/src/conf/capabilities.c
@@ -701,13 +701,14 @@ virCapabilitiesDomainDataLookup(virCapsPtr caps,
!virBufferCurrentContent(buf)[0])
virBufferAsprintf(buf, %s, _(any configuration));
if (virBufferCheckError(buf) 0) {
-virBufferContentAndReset(buf);
+virBufferFreeAndReset(buf);
goto error;
}
virReportError(VIR_ERR_INVALID_ARG,
_(could not find capabilities for %s),
- virBufferContentAndReset(buf));
+ virBufferCurrentContent(buf));
+virBufferFreeAndReset(buf);
goto error;
}
--
1.7.12.4
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH] util: fix memleak in virStorageSourceClear
snapshot and configFile are not freed, free them. Signed-off-by: Zhang Bo oscar.zhan...@huawei.com --- src/util/virstoragefile.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/util/virstoragefile.c b/src/util/virstoragefile.c index 9507ca1..6c3017c 100644 --- a/src/util/virstoragefile.c +++ b/src/util/virstoragefile.c @@ -2038,6 +2038,8 @@ virStorageSourceClear(virStorageSourcePtr def) VIR_FREE(def-path); VIR_FREE(def-volume); +VIR_FREE(def-snapshot); +VIR_FREE(def-configFile); virStorageSourcePoolDefFree(def-srcpool); VIR_FREE(def-driverName); virBitmapFree(def-features); -- 1.7.12.4 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 1/6] tests: fix some memleaks in tests
Signed-off-by: Zhang Bo oscar.zhan...@huawei.com --- tests/commandtest.c | 1 + tests/domaincapstest.c | 1 + tests/qemucommandutiltest.c | 1 + 3 files changed, 3 insertions(+) diff --git a/tests/commandtest.c b/tests/commandtest.c index 6400ea2..f001a39 100644 --- a/tests/commandtest.c +++ b/tests/commandtest.c @@ -1081,6 +1081,7 @@ static int test24(const void *unused ATTRIBUTE_UNUSED) if (pidfile) unlink(pidfile); VIR_FREE(pidfile); +VIR_FREE(prefix); virCommandFree(cmd); VIR_FORCE_CLOSE(newfd1); /* coverity[double_close] */ diff --git a/tests/domaincapstest.c b/tests/domaincapstest.c index f6a060e..ecefdb9 100644 --- a/tests/domaincapstest.c +++ b/tests/domaincapstest.c @@ -242,6 +242,7 @@ mymain(void) ret = -1; \ } else if (virtTestRun(Filename, test_virDomainCapsFormat, data) 0) \ ret = -1; \ +virObjectUnref(qemuCaps); \ } while (0) DO_TEST_QEMU(qemu_1.6.50-1, caps_1.6.50-1, /usr/bin/qemu-system-x86_64, diff --git a/tests/qemucommandutiltest.c b/tests/qemucommandutiltest.c index 8c52f02..bd457f8 100644 --- a/tests/qemucommandutiltest.c +++ b/tests/qemucommandutiltest.c @@ -66,6 +66,7 @@ testQemuCommandBuildObjectFromJSON(const void *opaque) cleanup: virJSONValueFree(val); VIR_FREE(result); +VIR_FREE(expect); return ret; } -- 1.7.12.4 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 0/6] fix a serial of memleaks
Zhang Bo (6): tests: fix some memleaks in tests util: fix memleak in virFindSCSIHostByPCI qemu: fix memleaks in qemuBuildCommandLine qemu: fix memleak in virCapabilitiesDomainDataLookup conf: fix memleak in virDomainNetIpParseXML conf: fix memleak in virDomainHostdevDefClear src/conf/capabilities.c | 5 +++-- src/conf/domain_conf.c | 18 ++ src/qemu/qemu_command.c | 2 ++ src/util/virutil.c | 3 +++ tests/commandtest.c | 1 + tests/domaincapstest.c | 1 + tests/qemucommandutiltest.c | 1 + 7 files changed, 21 insertions(+), 10 deletions(-) -- 1.7.12.4 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 3/6] qemu: fix memleaks in qemuBuildCommandLine
free boot_opts_str and boot_order_str both in normal and error paths. Signed-off-by: Zhang Bo oscar.zhan...@huawei.com --- src/qemu/qemu_command.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index 29b876e..a54f3a3 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -9266,6 +9266,7 @@ qemuBuildCommandLine(virConnectPtr conn, } } VIR_FREE(boot_opts_str); +VIR_FREE(boot_order_str); if (def-os.kernel) virCommandAddArgList(cmd, -kernel, def-os.kernel, NULL); @@ -10746,6 +10747,7 @@ qemuBuildCommandLine(virConnectPtr conn, error: VIR_FREE(boot_order_str); +VIR_FREE(boot_opts_str); virBufferFreeAndReset(boot_buf); virObjectUnref(cfg); /* free up any resources in the network driver -- 1.7.12.4 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 2/6] util: fix memleak in virFindSCSIHostByPCI
free buf in cleanup.
Signed-off-by: Zhang Bo oscar.zhan...@huawei.com
---
src/util/virutil.c | 3 +++
1 file changed, 3 insertions(+)
diff --git a/src/util/virutil.c b/src/util/virutil.c
index 79cdb7a..0426517 100644
--- a/src/util/virutil.c
+++ b/src/util/virutil.c
@@ -1815,6 +1815,8 @@ virFindSCSIHostByPCI(const char *sysfs_prefix,
if (virStrToLong_ui(buf, NULL, 10, read_unique_id) 0)
goto cleanup;
+VIR_FREE(buf);
+
if (read_unique_id != unique_id) {
VIR_FREE(unique_path);
continue;
@@ -1829,6 +1831,7 @@ virFindSCSIHostByPCI(const char *sysfs_prefix,
VIR_FREE(unique_path);
VIR_FREE(host_link);
VIR_FREE(host_path);
+VIR_FREE(buf);
return ret;
}
--
1.7.12.4
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] question: any conference available?
Besides the maillist and IRC, are there any other means of communication available for libvirt? I'm much concerned about scheduled conference, including online conferences(weekly? monthly?) or face-to-face conference. so that developers from all around could join together to share about libvirt's roadmap, release plan, etc. Thank you for your reply in advance. -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 5/6] conf: fix memleak in virDomainNetIpParseXML
use cleanup instead of error, so that the allocated strings could also get freed
when there's no error.
Signed-off-by: Zhang Bo oscar.zhan...@huawei.com
---
src/conf/domain_conf.c | 16 +---
1 file changed, 9 insertions(+), 7 deletions(-)
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index 41963cc..8350fe7 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -5129,6 +5129,7 @@ virDomainNetIpParseXML(xmlNodePtr node)
char *familyStr = NULL;
int family = AF_UNSPEC;
char *address = NULL;
+int ret = -1;
if (!(prefixStr = virXMLPropString(node, prefix)) ||
(virStrToLong_ui(prefixStr, NULL, 10, prefixValue) 0)) {
@@ -5139,7 +5140,7 @@ virDomainNetIpParseXML(xmlNodePtr node)
if (!(address = virXMLPropString(node, address))) {
virReportError(VIR_ERR_INVALID_ARG, %s,
_(Missing network address));
-goto error;
+goto cleanup;
}
familyStr = virXMLPropString(node, family);
@@ -5151,24 +5152,25 @@ virDomainNetIpParseXML(xmlNodePtr node)
family = virSocketAddrNumericFamily(address);
if (VIR_ALLOC(ip) 0)
-goto error;
+goto cleanup;
if (virSocketAddrParse(ip-address, address, family) 0) {
virReportError(VIR_ERR_INVALID_ARG,
_(Failed to parse IP address: '%s'),
address);
-goto error;
+goto cleanup;
}
ip-prefix = prefixValue;
-return ip;
+ret = 0;
- error:
+ cleanup:
VIR_FREE(prefixStr);
VIR_FREE(familyStr);
VIR_FREE(address);
-VIR_FREE(ip);
-return NULL;
+if (ret)
+VIR_FREE(ip);
+return ip;
}
static int
--
1.7.12.4
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 0/2] tests: fix memleak in qemumonitorjsontest
the free callback should be qemuMonitorChardevInfoFree rather than just 'free' when virHashCreate'ing the chardevInfo hash. Zhang Bo (2): qemu: make qemuMonitorChardevInfoFree non-static tests: free ChardevInfo correctly in qemumonitorjsontest src/qemu/qemu_monitor.c | 2 +- src/qemu/qemu_monitor.h | 1 + tests/qemumonitorjsontest.c | 4 ++-- 3 files changed, 4 insertions(+), 3 deletions(-) -- 1.7.12.4 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 1/2] qemu: make qemuMonitorChardevInfoFree non-static
It would be used in qemumonitorjsontest, thus we make it non-static.
Signed-off-by: Zhang Bo oscar.zhan...@huawei.com
Signed-off-by: Zhou Yimin zhouyi...@huawei.com
---
src/qemu/qemu_monitor.c | 2 +-
src/qemu/qemu_monitor.h | 1 +
2 files changed, 2 insertions(+), 1 deletion(-)
diff --git a/src/qemu/qemu_monitor.c b/src/qemu/qemu_monitor.c
index 1e7d2ef..c2a9028 100644
--- a/src/qemu/qemu_monitor.c
+++ b/src/qemu/qemu_monitor.c
@@ -2812,7 +2812,7 @@ qemuMonitorQueryRxFilter(qemuMonitorPtr mon, const char
*alias,
}
-static void
+void
qemuMonitorChardevInfoFree(void *data,
const void *name ATTRIBUTE_UNUSED)
{
diff --git a/src/qemu/qemu_monitor.h b/src/qemu/qemu_monitor.h
index cd4cc66..ba6e4e5 100644
--- a/src/qemu/qemu_monitor.h
+++ b/src/qemu/qemu_monitor.h
@@ -649,6 +649,7 @@ struct _qemuMonitorChardevInfo {
char *ptyPath;
virDomainChrDeviceState state;
};
+void qemuMonitorChardevInfoFree(void *data, const void *name);
int qemuMonitorGetChardevInfo(qemuMonitorPtr mon,
virHashTablePtr *retinfo);
--
1.7.12.4
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 2/2] tests: free ChardevInfo correctly in qemumonitorjsontest
the free callback should be qemuMonitorChardevInfoFree rather than just 'free' when virHashCreate'ing the chardevInfo hash. Signed-off-by: Zhang Bo oscar.zhan...@huawei.com Signed-off-by: Zhou Yimin zhouyi...@huawei.com --- tests/qemumonitorjsontest.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tests/qemumonitorjsontest.c b/tests/qemumonitorjsontest.c index f729c7c..9296668 100644 --- a/tests/qemumonitorjsontest.c +++ b/tests/qemumonitorjsontest.c @@ -28,7 +28,7 @@ #include virerror.h #include virstring.h #include cpu/cpu.h - +#include qemu/qemu_monitor.h #define VIR_FROM_THIS VIR_FROM_NONE @@ -1782,7 +1782,7 @@ testQemuMonitorJSONqemuMonitorJSONGetChardevInfo(const void *data) if (!test) return -1; -if (!(info = virHashCreate(32, (virHashDataFree) free)) || +if (!(info = virHashCreate(32, qemuMonitorChardevInfoFree)) || !(expectedInfo = virHashCreate(32, NULL))) goto cleanup; -- 1.7.12.4 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH] typo: fix typo error in virLXCControllerSetupResourceLimits
iff - if
rather then - rather than
Signed-off-by: YueWenyuan yueweny...@huawei.com
---
src/lxc/lxc_controller.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/src/lxc/lxc_controller.c b/src/lxc/lxc_controller.c
index e144c2d..7ad9d50 100644
--- a/src/lxc/lxc_controller.c
+++ b/src/lxc/lxc_controller.c
@@ -745,9 +745,9 @@ static int
virLXCControllerSetupResourceLimits(virLXCControllerPtr ctrl)
if (mode == VIR_DOMAIN_NUMATUNE_MEM_STRICT
virCgroupControllerAvailable(VIR_CGROUP_CONTROLLER_CPUSET)) {
-/* Use virNuma* API iff necessary. Once set and child is exec()-ed,
+/* Use virNuma* API if necessary. Once set and child is exec()-ed,
* there's no way for us to change it. Rely on cgroups (if available
- * and enabled in the config) rather then virNuma*. */
+ * and enabled in the config) rather than virNuma*. */
VIR_DEBUG(Relying on CGroups for memory binding);
} else {
--
1.7.12.4
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] qemu: lock: unlock vm during qemuBuildCommandLine
On 2015/4/23 17:46, Michal Privoznik wrote:
On 23.04.2015 11:32, Peter Krempa wrote:
On Thu, Apr 23, 2015 at 11:19:34 +0200, Michal Privoznik wrote:
On 23.04.2015 10:30, Peter Krempa wrote:
On Thu, Apr 23, 2015 at 11:44:49 +0800, zhang bo wrote:
The function qemuBuildCommandLine() may take a long time, for example
if we configure tens of vifs for the guest, each may cost hundrands of
milliseconds to create tap dev, senconds in total. Thus, unlock vm
before calling it.
Signed-off-by: Zhang Bo oscar.zhan...@huawei.com
Signed-off-by: Zhou Yimin zhouyi...@huawei.com
---
src/qemu/qemu_process.c | 6 +-
1 file changed, 5 insertions(+), 1 deletion(-)
diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c
index 753afe8..d1aaaec 100644
--- a/src/qemu/qemu_process.c
+++ b/src/qemu/qemu_process.c
@@ -4628,14 +4628,18 @@ int qemuProcessStart(virConnectPtr conn,
}
VIR_DEBUG(Building emulator command line);
+virObjectUnlock(vm);
if (!(cmd = qemuBuildCommandLine(conn, driver, vm-def,
priv-monConfig,
priv-monJSON, priv-qemuCaps,
migrateFrom, stdin_fd, snapshot,
vmop,
buildCommandLineCallbacks, false,
qemuCheckFips(),
priv-autoNodeset,
- nnicindexes, nicindexes)))
+ nnicindexes, nicindexes))) {
+virObjectLock(vm);
goto cleanup;
+}
+virObjectLock(vm);
Why do you need to unlock the object? The VM is starting at this point
so you won't be able to save any time since APIs will either be blocked
by a job or by the fact that the VM was not started.
Not true. We still allow QUERY jobs, or APIs that lock a domain object
but don't necessarily take a job. For instance, if in one thread I'm
doing virDomainGetOSType(), virDomainGetInfo(), ... (essentially 'virsh
dominfo') while the other just starts the domain. I don't see a reason
why virDomainGetOSType() should wait for the domain to be started up.
Domain state should have no affect on the guest OS type, should it?
OK that is technically right. I wanted to point out that most of the
stats are available only for online machines or it shouldn't be much of
a problem to dealy the delivery.
Your example certainly doesn't illustrate why the delay to format the
command line should be a problem when using libvirt. Or are you polling
virDomainGetOSType every millisecond?
I am curious to see why the delay would be a problem.
Yep, I'm too. So far we don't really care much about libvirt response
time (otherwise our critical sections would be much shorter), but maybe
it's an issue for somebody.
The specific semantic is:
migrating multiple guests simultaneously, downtime of each guest will add up,
even to an unacceptable value.
1 suppose the downtime is 2 seconds when we migrate only 1 guest at one time.
2 suppose it costs 0.5sec to create each tapDev, and each guest has 20 vifs,
that's 10 seconds for qemuBuildCommandLine.
3 now, we migrate 10 guest simultaneously, the downtime of the guests will vary
from
seconds to even 100 seconds.
The reason for the problem is that:
1 guestA locks vm while creating each tapDev(virNetDevTapCreate) in
qemuBuildCommandLine(), for 10seconds
2 guestB calls qemuMigrationPrepareAny-*virDomainObjListAdd* to get its vm
object, which locks 'doms'
and waits for the vm lock.
3 doms will be locked until guestA unlock its vm, we say that's 10 seconds.
4 guestC calls qemuDomainMigrateFinish3-virDomainObjListFindByName, which
tries to lock doms. because it's
now locked by guestB, guestC blocks here, and it can't be unpaused for at
least 10 seconds.
5 then comes to guestD, guestE, guestF, etc, the downtime will be added up, to
even 50 seconds or more.
6 the command 'virsh list' is blocked as well.
Thus, we think the problem must be solved.
On the other hand, I don't think we can just lock and unlock the domain
object as we please. qemuBuildCommandLine is a very complex function and
as such it calls many others. Some of them may rely on the fact, that
the object is locked by caller.
Well, you definitely can't since almost everything in there is accessing
vm-def. Locking semantics would be broken right in the line after @vm
was unlocked by dereferencing it.
Well, anything that would change a domain definition should grap a
MODIFY job. But such jobs are serialized, so even if we unlock the
domain we should be okay to still access vm-def. What I am more worried
about are all the small functions that interact with system or
something. Currently they are serialized by @vm lock, but one we remove
it ...
Michal
.
After the discussion above, maybe it's better to move virNetDevTapCreate()
prior to qemuBuildCommandLine(), what do
you think about
Re: [libvirt] [PATCH] qemu: lock: unlock vm during qemuBuildCommandLine
On 2015/4/23 19:06, Daniel P. Berrange wrote: On Thu, Apr 23, 2015 at 07:00:21PM +0800, zhang bo wrote: The reason for the problem is that: 1 guestA locks vm while creating each tapDev(virNetDevTapCreate) in qemuBuildCommandLine(), for 10seconds 2 guestB calls qemuMigrationPrepareAny-*virDomainObjListAdd* to get its vm object, which locks 'doms' and waits for the vm lock. 3 doms will be locked until guestA unlock its vm, we say that's 10 seconds. 4 guestC calls qemuDomainMigrateFinish3-virDomainObjListFindByName, which tries to lock doms. because it's Ok, this is the real core problem - FindByName has a bad impl that requires iterating over every single guest. Unfortunately due to the design of the migration API we can't avoid this call, but we could add a second hash table of name - virDomainObj so we make it O(1) and lock-less. I got a question: shall we add an object (similar to doms) and lock it while searching the vm in the new hash table? If so, the problem may still exist. now locked by guestB, guestC blocks here, and it can't be unpaused for at least 10 seconds. 5 then comes to guestD, guestE, guestF, etc, the downtime will be added up, to even 50 seconds or more. 6 the command 'virsh list' is blocked as well. Thus, we think the problem must be solved. Regards, Daniel -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 0/5] Make virDomainObjListFindByName run in O(1)
On 2015/4/24 1:14, Michal Privoznik wrote: As discussed here: https://www.redhat.com/archives/libvir-list/2015-April/msg01135.html Michal Privoznik (5): virDomainObjListAddLocked: s/false/NULL/ for @oldDef virDomainObjListNew: Use virObjectFreeHashData Introduce virDomainObjEndAPI virDomainObjListFindByName: Return referenced object virDomainObjList: Introduce yet another hash table src/bhyve/bhyve_driver.c | 3 +- src/conf/domain_conf.c | 83 +++ src/conf/domain_conf.h | 2 + src/libvirt_private.syms | 1 + src/libxl/libxl_driver.c | 10 ++--- src/lxc/lxc_driver.c | 3 +- src/openvz/openvz_driver.c | 11 +++-- src/parallels/parallels_driver.c | 3 +- src/qemu/qemu_domain.c | 7 +-- src/qemu/qemu_driver.c | 14 ++ src/test/test_driver.c | 93 +--- src/uml/uml_driver.c | 15 +++ 12 files changed, 110 insertions(+), 135 deletions(-) BTW, we just dealt with virDomainObjListFindByName() here, without caring about virDomainObjListFindByID(). virDomainObjListFindByID() is now called by umlDomainDestroyFlags() and umlDomainShutdownFlags(), if we simultaneously shutdown multiple vms on hypervisor UML, then similar problem comes out: some guest maybe unable to shutdown immediately, until other guests get shutoff, and even 'virsh list' blocks. so, shall we: 1 don't take such problem as a problem 2 use virDomainObjListFindByName() instead of virDomainObjListFindByID() there. 3 add a third hash table for domid If we use virDomainObjListFindByName() there, and forbids developers from using **ByID(), it seems unacceptable for any developers. If we add a third hash table, it may become not easy to maintain the high-complexity codes. Is there a better solution other than adding more hash tables? -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 0/5] Make virDomainObjListFindByName run in O(1)
On 2015/4/24 1:14, Michal Privoznik wrote: As discussed here: https://www.redhat.com/archives/libvir-list/2015-April/msg01135.html Michal Privoznik (5): virDomainObjListAddLocked: s/false/NULL/ for @oldDef virDomainObjListNew: Use virObjectFreeHashData Introduce virDomainObjEndAPI virDomainObjListFindByName: Return referenced object virDomainObjList: Introduce yet another hash table src/bhyve/bhyve_driver.c | 3 +- src/conf/domain_conf.c | 83 +++ src/conf/domain_conf.h | 2 + src/libvirt_private.syms | 1 + src/libxl/libxl_driver.c | 10 ++--- src/lxc/lxc_driver.c | 3 +- src/openvz/openvz_driver.c | 11 +++-- src/parallels/parallels_driver.c | 3 +- src/qemu/qemu_domain.c | 7 +-- src/qemu/qemu_driver.c | 14 ++ src/test/test_driver.c | 93 +--- src/uml/uml_driver.c | 15 +++ 12 files changed, 110 insertions(+), 135 deletions(-) Great. I tested the patch series, the migration downtime problem disappears. -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH] qemu: Don't fail to reboot domains with unresponsive agent
just as what b8e25c35d7f80a2fadc0e51e95318e39db3d1687 did,
we fall back to the ACPI method when the guest agent is unresponsive in
qemuDomainReboot.
Signed-off-by: YueWenyuan yueweny...@huawei.com
Signed-off-by: Zhang Bo oscar.zhan...@huawei.com
---
src/qemu/qemu_driver.c | 67 +-
1 file changed, 34 insertions(+), 33 deletions(-)
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index 6fc9696..964a9c5 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -2002,21 +2002,14 @@ qemuDomainReboot(virDomainPtr dom, unsigned int flags)
virDomainObjPtr vm;
int ret = -1;
qemuDomainObjPrivatePtr priv;
-bool useAgent = false;
+bool useAgent = false, agentRequested, acpiRequested;
bool isReboot = true;
+bool agentForced;
int agentFlag = QEMU_AGENT_SHUTDOWN_REBOOT;
virCheckFlags(VIR_DOMAIN_REBOOT_ACPI_POWER_BTN |
VIR_DOMAIN_REBOOT_GUEST_AGENT, -1);
-/* At most one of these two flags should be set. */
-if ((flags VIR_DOMAIN_REBOOT_ACPI_POWER_BTN)
-(flags VIR_DOMAIN_REBOOT_GUEST_AGENT)) {
-virReportInvalidArg(flags, %s,
-_(flags for acpi power button and guest agent are
mutually exclusive));
-return -1;
-}
-
if (!(vm = qemuDomObjFromDomain(dom)))
goto cleanup;
@@ -2028,38 +2021,25 @@ qemuDomainReboot(virDomainPtr dom, unsigned int flags)
}
priv = vm-privateData;
+agentRequested = flags VIR_DOMAIN_REBOOT_GUEST_AGENT;
+acpiRequested = flags VIR_DOMAIN_REBOOT_ACPI_POWER_BTN;
if (virDomainRebootEnsureACL(dom-conn, vm-def, flags) 0)
goto cleanup;
-if ((flags VIR_DOMAIN_REBOOT_GUEST_AGENT) ||
-(!(flags VIR_DOMAIN_REBOOT_ACPI_POWER_BTN)
- priv-agent))
+/* Prefer agent unless we were requested to not to. */
+if (agentRequested || (!flags priv-agent))
useAgent = true;
-if (!useAgent) {
-#if WITH_YAJL
-if (virQEMUCapsGet(priv-qemuCaps, QEMU_CAPS_MONITOR_JSON)) {
-if (!virQEMUCapsGet(priv-qemuCaps, QEMU_CAPS_NO_SHUTDOWN)) {
-virReportError(VIR_ERR_CONFIG_UNSUPPORTED, %s,
- _(Reboot is not supported with this QEMU
binary));
-goto cleanup;
-}
-} else {
-#endif
-virReportError(VIR_ERR_OPERATION_INVALID, %s,
- _(Reboot is not supported without the JSON
monitor));
-goto cleanup;
-#if WITH_YAJL
-}
-#endif
-}
-
if (qemuDomainObjBeginJob(driver, vm, QEMU_JOB_MODIFY) 0)
goto cleanup;
-if (useAgent !qemuDomainAgentAvailable(vm, true))
-goto endjob;
+agentForced = agentRequested !acpiRequested;
+if (useAgent !qemuDomainAgentAvailable(vm, true)) {
+if (agentForced)
+goto endjob;
+useAgent = false;
+}
if (!virDomainObjIsActive(vm)) {
virReportError(VIR_ERR_OPERATION_INVALID,
@@ -2071,7 +2051,28 @@ qemuDomainReboot(virDomainPtr dom, unsigned int flags)
qemuDomainObjEnterAgent(vm);
ret = qemuAgentShutdown(priv-agent, agentFlag);
qemuDomainObjExitAgent(vm);
-} else {
+}
+
+/* If we are not enforced to use just an agent, try ACPI
+ * shutdown as well in case agent did not succeed.
+ */
+if ((!useAgent) ||
+(ret 0 (acpiRequested || !flags))) {
+#if WITH_YAJL
+if (virQEMUCapsGet(priv-qemuCaps, QEMU_CAPS_MONITOR_JSON)) {
+if (!virQEMUCapsGet(priv-qemuCaps, QEMU_CAPS_NO_SHUTDOWN)) {
+virReportError(VIR_ERR_CONFIG_UNSUPPORTED, %s,
+ _(ACPI reboot is not supported with this QEMU
binary));
+goto endjob;
+}
+} else {
+#endif
+virReportError(VIR_ERR_OPERATION_INVALID, %s,
+ _(ACPI reboot is not supported without the JSON
monitor));
+goto endjob;
+#if WITH_YAJL
+}
+#endif
qemuDomainObjEnterMonitor(driver, vm);
ret = qemuMonitorSystemPowerdown(priv-mon);
if (qemuDomainObjExitMonitor(driver, vm) 0)
--
1.7.12.4
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH] qemu: lock: unlock vm during qemuBuildCommandLine
The function qemuBuildCommandLine() may take a long time, for example
if we configure tens of vifs for the guest, each may cost hundrands of
milliseconds to create tap dev, senconds in total. Thus, unlock vm
before calling it.
Signed-off-by: Zhang Bo oscar.zhan...@huawei.com
Signed-off-by: Zhou Yimin zhouyi...@huawei.com
---
src/qemu/qemu_process.c | 6 +-
1 file changed, 5 insertions(+), 1 deletion(-)
diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c
index 753afe8..d1aaaec 100644
--- a/src/qemu/qemu_process.c
+++ b/src/qemu/qemu_process.c
@@ -4628,14 +4628,18 @@ int qemuProcessStart(virConnectPtr conn,
}
VIR_DEBUG(Building emulator command line);
+virObjectUnlock(vm);
if (!(cmd = qemuBuildCommandLine(conn, driver, vm-def, priv-monConfig,
priv-monJSON, priv-qemuCaps,
migrateFrom, stdin_fd, snapshot, vmop,
buildCommandLineCallbacks, false,
qemuCheckFips(),
priv-autoNodeset,
- nnicindexes, nicindexes)))
+ nnicindexes, nicindexes))) {
+virObjectLock(vm);
goto cleanup;
+}
+virObjectLock(vm);
/* now that we know it is about to start call the hook if present */
if (virHookPresent(VIR_HOOK_DRIVER_QEMU)) {
--
1.7.12.4
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] qemu: lifecycle: guest get rebooted after reboot and shutdown
On 2015/4/17 17:54, Michal Privoznik wrote: On 17.04.2015 02:43, zhang bo wrote: Maybe I should have been more specific when requiring you to send the patch. If you look at 'git log' you'll see this commit message diverges from the rest. I've fixed it, ACKed and pushed. Michal Sorry for the inconvenience I brought in and Thank you very much!! :) Oscar -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] util: log: How about online change logLevel?
Sometimes, when we encounter with a problem, we need to set libvirtd log level to DEBUG to dig detailed information. But changing log level requires us to *restart* libvirtd. 1) #vim /etc/libvirt/libvirtd.conf log_level = 1 2) *service libvirtd restart* We think that it makes sense to develop an API to *online* change log level, without restarting libvirtd. What's your opinion? If it's necessary, I'd like to post a patch here. -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] qemu: lifecycle: reboot + shutdown, unexpected vm status.
Steps:
1 virsh reboot guest1 --mode=acpi
2 virsh shutdown guest1 --mode=agent
Expected result:
As the SHUTDOWN job is after REBOOT, we expected the guest to be *shutoff*.
(Do you think so?)
Exacted result:
After the 2 steps above, the guest got *rebooted*.
The reason to this problem:
1 in qemuDomainReboot(mode acpi), it sets priv-fakeReboot to 1.
2 after shutdown/reboot, qemu monitor IO trigged
qemuProcessHandleShutdown(), which finds that priv-fakeReboot is 1, and reboot
the guest.
Root Cause of the problem:
After further look into the problem, We found that the design of
acpi/agent shutdown/reboot seems a little chaotic.
---
sheet1 who sets fakeReboot
---
shutdown reboot
acpiY(0) Y(1)
agent N N
It's apparently, *acpi-mode* jobs set fakeReboot.
---
sheet2 who needs to check fakeReboot(qemuProcessHandleShutdown())
---
shutdown reboot
acpiY Y
agent *Y* *N*
Things become a little odd here. only agent-mode reboot doesn't check
fakeReboot.
We can tell from the above 2 sheets, that they're not consistent.
*Agent-mode shutdown needs to check fakeReboot(trigger by SHUTDOWN
monitorIO event), while it didn't set it before.*
The chaos is not caused by libvirtd, it's a systematic problem, including
guest os and qemu. (guest os writes ACPI, triggers qemu, qemu then triggers
libvirtd)
My Solution:
A simple solution is to make the 1st sheet consistent to the 2nd sheet,
which is:
---
sheet3 who should set fakeReboot:
---
shutdown reboot
acpiY(0) Y(1)
agent *Y(0)* N
---
we let agent-mode shutdown set fakeReboot to 0.
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index 7eb5a7d..c751dcf 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -1959,6 +1959,8 @@ static int qemuDomainShutdownFlags(virDomainPtr dom,
unsigned int flags)
goto endjob;
}
+qemuDomainSetFakeReboot(driver, vm, isReboot);
+
if (useAgent) {
qemuDomainObjEnterAgent(vm);
ret = qemuAgentShutdown(priv-agent, agentFlag);
@@ -1970,7 +1972,6 @@ static int qemuDomainShutdownFlags(virDomainPtr dom,
unsigned int flags)
*/
if (!useAgent ||
(ret 0 (acpiRequested || !flags))) {
-qemuDomainSetFakeReboot(driver, vm, isReboot);
/* Even if agent failed, we have to check if guest went away
* by itself while our locks were down. */
--
Discussion:
Although the solution above seems to have solved the problem, but it seems
a little awkward to have sheets like this.
Maybe we should let sheet2 consistent with sheet1, rather than letting
sheet1 consistent to sheet2. But It's too difficult to realize that, seems
impossible
So, is there a better way to solve this problem? or, shall I commit this
patch?
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] util: client: shall libvirtd record task histories?
On 2015/4/10 15:54, Jiri Denemark wrote: On Wed, Apr 08, 2015 at 15:40:36 +0800, zhang bo wrote: We recently encountered a problem: 1) migrate a domain 2) the client unexpectedly got *crashed* (let's take it as virsh command) 3) *libvirtd still kept migrating the domain* 4) after it's restarted, the client didn't know the guest is still migrating. The problem is that libvirtd and the client has different view of the task state. After migration, the client may wrongly think that something's wrong that the domain got unexpectedly migrated. In my opinion, libvirtd should just *execute* tasks, like the hands of a human, while clients should be the brain to *schedule and remember* tasks. So, In order to avoid this problem,we should let the client record all the taskes somewhere, and reload the states after its restart. the client may cancel or continue the task as it wishes. Libvirtd should not record the task status. Not really. It's libvirtd, the daemon, which has to remember everything. It manages the state of all domains running on a host and synchronizes all clients that want to change state of the domains. Remember, even if a client is not restarted, domains my unexpectedly migrate somewhere else because another client might have asked for it. That said, if you're implementing a higher management layer which manages domains using libvirt and you know it is going to be the only client talking directly to libvirt, you can remember the state there if you want. However, it's not something libvirt itself should or could do. But you will most likely need to synchronize the state with libvirtd in case the client is restarted. Even libvirtd has to synchronize its internal state with all running QEMU processes when it is restarted because the state might have changed. Jirka . Thank you Jirka. Let's go a step further, suppose that the client doesn't crash at step 2), it's just disconnected to libvirtd at src side. 1) client(nova) calls virDomainMigrateToURI2() to migrate a guest 2) libvirtd at src side connects to libvirtd at dest side. 3) Unfortunately, somehow, client(nova) gets disconnected to libvirtd while migrating the guest. 4) the API virDomainMigrateToURI2() returns with error in client(nova) 5) but libvirtd doesn't aware that the connection to client is broken, and keeps migrating the guest to dest. 6) the guest is migrated to the dest side eventually. 7) Because the nova at src side thinks migration is not successed as step 4), the nova at the dest will consider the migrated-in guest as an unexpected running guest, and will shut it down. The guest disappears at last, due to the previous disconnection of libvirtd client and server. Even though libvirtd remembers everything, the client at dest side still wrongly killed the guest after migration. So, how to solve this problem? Shall libvirtd keep watching its clients' connection, and cancel running jobs concerning the disconnected client immediately after the client disconnects? -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH] Add source check before attaching device
Source device/file is not unique now, we should check it when attach device.
Signed-off-by: YueWenyuan yueweny...@huawei.com
Signed-off-by: Zhang Bo oscar.zhan...@huawei.com
---
src/conf/domain_conf.c | 15 +++
src/conf/domain_conf.h | 1 +
src/libvirt_private.syms | 1 +
src/qemu/qemu_driver.c | 12 +++-
src/qemu/qemu_hotplug.c | 42 +++---
5 files changed, 55 insertions(+), 16 deletions(-)
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index 58b98c6..3fd729c 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -18191,6 +18191,21 @@ virDomainControllerDefFormat(virBufferPtr buf,
int
+virDomainFSIndexBySrc(virDomainDefPtr def, const char *src)
+{
+virDomainFSDefPtr fs;
+size_t i;
+
+for (i = 0; i def-nfss; i++) {
+fs = def-fss[i];
+if (STREQ(fs-src, src))
+return i;
+}
+return -1;
+}
+
+
+int
virDomainFSIndexByName(virDomainDefPtr def, const char *name)
{
virDomainFSDefPtr fs;
diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h
index e6fa3c9..e23f289 100644
--- a/src/conf/domain_conf.h
+++ b/src/conf/domain_conf.h
@@ -2804,6 +2804,7 @@ int virDiskNameToBusDeviceIndex(virDomainDiskDefPtr disk,
virDomainFSDefPtr virDomainGetFilesystemForTarget(virDomainDefPtr def,
const char *target);
int virDomainFSInsert(virDomainDefPtr def, virDomainFSDefPtr fs);
+int virDomainFSIndexBySrc(virDomainDefPtr def, const char *src);
int virDomainFSIndexByName(virDomainDefPtr def, const char *name);
virDomainFSDefPtr virDomainFSRemove(virDomainDefPtr def, size_t i);
diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
index 7166283..611c0d4 100644
--- a/src/libvirt_private.syms
+++ b/src/libvirt_private.syms
@@ -274,6 +274,7 @@ virDomainDiskSourceIsBlockType;
virDomainEmulatorPinAdd;
virDomainEmulatorPinDel;
virDomainFSDefFree;
+virDomainFSIndexBySrc;
virDomainFSIndexByName;
virDomainFSInsert;
virDomainFSRemove;
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index cbb6e1b..3b187f0 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -7953,6 +7953,11 @@ qemuDomainAttachDeviceConfig(virQEMUCapsPtr qemuCaps,
_(target %s already exists), disk-dst);
return -1;
}
+if (virDomainDiskIndexByName(vmdef, disk-src-path, true) = 0) {
+virReportError(VIR_ERR_OPERATION_INVALID,
+ _(source %s already exists), disk-src-path);
+return -1;
+}
if (qemuCheckDiskConfig(disk) 0)
return -1;
if (virDomainDiskInsert(vmdef, disk))
@@ -8035,7 +8040,12 @@ qemuDomainAttachDeviceConfig(virQEMUCapsPtr qemuCaps,
fs = dev-data.fs;
if (virDomainFSIndexByName(vmdef, fs-dst) = 0) {
virReportError(VIR_ERR_OPERATION_INVALID,
- %s, _(Target already exists));
+ _(Target %s already exists), fs-dst);
+return -1;
+}
+if (virDomainFSIndexBySrc(vmdef, fs-src) = 0) {
+virReportError(VIR_ERR_OPERATION_INVALID,
+ _(Source %s already exists), fs-src);
return -1;
}
diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c
index 2f0549e..5dd2453 100644
--- a/src/qemu/qemu_hotplug.c
+++ b/src/qemu/qemu_hotplug.c
@@ -315,12 +315,34 @@ qemuDomainCheckEjectableMedia(virQEMUDriverPtr driver,
}
static int
+qemuDomainCheckDiskDeviceExists(virDomainObjPtr vm,
+ virDomainDiskDefPtr disk)
+{
+int ret = -1;
+size_t i;
+for (i = 0; i vm-def-ndisks; i++) {
+if (STREQ(vm-def-disks[i]-dst, disk-dst)) {
+virReportError(VIR_ERR_OPERATION_FAILED,
+ _(target %s already exists), disk-dst);
+return ret;
+}
+if (disk-src vm-def-disks[i]-src
+disk-src-path vm-def-disks[i]-src-path
+STREQ(vm-def-disks[i]-src-path, disk-src-path)) {
+virReportError(VIR_ERR_OPERATION_FAILED,
+ _(source %s already exists), disk-src-path);
+return ret;
+}
+}
+return 0;
+}
+
+static int
qemuDomainAttachVirtioDiskDevice(virConnectPtr conn,
virQEMUDriverPtr driver,
virDomainObjPtr vm,
virDomainDiskDefPtr disk)
{
-size_t i;
int ret = -1;
const char* type = virDomainDiskBusTypeToString(disk-bus);
qemuDomainObjPrivatePtr priv = vm-privateData;
@@ -338,13 +360,8 @@ qemuDomainAttachVirtioDiskDevice(virConnectPtr conn,
disk-info.type = VIR_DOMAIN_DEVICE_ADDRESS_TYPE_VIRTIO_S390;
}
-for (i = 0; i vm-def-ndisks; i++) {
-if (STREQ(vm-def-disks[i]-dst, disk-dst
Re: [libvirt] [PATCH] Add source check before attaching device
On 2015/4/16 17:44, Jiri Denemark wrote: On Thu, Apr 16, 2015 at 16:51:52 +0800, zhang bo wrote: Source device/file is not unique now, we should check it when attach device. The check is vary fragile and unreliable. It would be very hard (perhaps even impossible) to really detect that the two disks/filesystems point to the same source, which means libvirt would not really prevent anyone from attaching a single source twice. Moreover, the code as written would incorrectly refuse to attach disks that do not point to the same source (network disks, e.g.). But anyway, I don't think libvirt should be doing this in general, so NACK to this idea. Moreover, you should be able to achieve the same goal (and even more) by using a locking driver. Jirka That's true! we didn't think of that. Thank you Jirka. oscar -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH] qemu: lifecycle: guest get rebooted after reboot and shutdown
Steps:
1 virsh reboot guest1 --mode=acpi
2 virsh shutdown guest1 --mode=agent
Expected result:
As the SHUTDOWN job is after REBOOT, we expected the guest to be *shutoff*.
Exacted result:
After the 2 steps above, the guest got *rebooted*.
The reason to this problem:
1 in qemuDomainReboot(mode acpi), it sets priv-fakeReboot to 1.
2 after shutdown/reboot, qemu monitor IO trigged
qemuProcessHandleShutdown(), which finds that priv-fakeReboot is 1, and reboot
the guest.
Root Cause of the problem:
After further look into the problem, We found that the design of
acpi/agent shutdown/reboot seems a little chaotic.
---
sheet1 who sets fakeReboot
---
shutdown reboot
acpiY(0) Y(1)
agent N N
It's apparently, *acpi-mode* jobs set fakeReboot.
---
sheet2 who needs to check fakeReboot(qemuProcessHandleShutdown())
---
shutdown reboot
acpiY Y
agent *Y* *N*
Things become a little odd here. only agent-mode reboot doesn't check
fakeReboot.
We can tell from the above 2 sheets, that they're not consistent.
*Agent-mode shutdown checks fakeReboot(trigger by SHUTDOWN monitorIO
event), while it didn't set it before.*
The chaos is not caused by libvirtd, it's a systematic problem, including
guest os and qemu. (guest os writes ACPI, triggers qemu, qemu then triggers
libvirtd)
Solution:
A simple solution is to make the 1st sheet consistent to the 2nd sheet,
which is:
---
sheet3 who should set fakeReboot:
---
shutdown reboot
acpiY(0) Y(1)
agent *Y(0)* N
---
we let agent-mode shutdown set fakeReboot to 0.
Signed-off-by: Zhang Bo oscar.zhan...@huawei.com
Signed-off-by: Wang Yufei james.wangyu...@huawei.com
---
src/qemu/qemu_driver.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index 7eb5a7d..c751dcf 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -1959,6 +1959,8 @@ static int qemuDomainShutdownFlags(virDomainPtr dom,
unsigned int flags)
goto endjob;
}
+qemuDomainSetFakeReboot(driver, vm, isReboot);
+
if (useAgent) {
qemuDomainObjEnterAgent(vm);
ret = qemuAgentShutdown(priv-agent, agentFlag);
@@ -1970,7 +1972,6 @@ static int qemuDomainShutdownFlags(virDomainPtr dom,
unsigned int flags)
*/
if (!useAgent ||
(ret 0 (acpiRequested || !flags))) {
-qemuDomainSetFakeReboot(driver, vm, isReboot);
/* Even if agent failed, we have to check if guest went away
* by itself while our locks were down. */
--
1.7.12.4
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] util: client: shall libvirtd record task histories?
We recently encountered a problem: 1) migrate a domain 2) the client unexpectedly got *crashed* (let's take it as virsh command) 3) *libvirtd still kept migrating the domain* 4) after it's restarted, the client didn't know the guest is still migrating. The problem is that libvirtd and the client has different view of the task state. After migration, the client may wrongly think that something's wrong that the domain got unexpectedly migrated. In my opinion, libvirtd should just *execute* tasks, like the hands of a human, while clients should be the brain to *schedule and remember* tasks. So, In order to avoid this problem,we should let the client record all the taskes somewhere, and reload the states after its restart. the client may cancel or continue the task as it wishes. Libvirtd should not record the task status. What's your opinions? thanks in advance. -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] qemu: migration: continuously sending and receiving ARP packets guest mistakenly thinks there's another guest with the same IP.
Problem Description: Live-migrate a guest, which has a tap device and continuously sends and receives ARP packets, it would mistakenly think there's another guest with the same IP, immedially after migration. The steps to reproduce the problem: 1 define and start a domain with its network configured as: interface type='bridge' mac address='52:54:00:7d:b0:af'/ source bridge='br0'/ virtualport type='openvswitch' parameters interfaceid='e4ad3dbb-7808-4175-83ee-ee0cba1c5456'/ /virtualport model type='virtio'/ driver name='vhost' queues='4'/ address type='pci' domain='0x' bus='0x00' slot='0x03' function='0x0'/ /interface 2 The guest sends ARP packets continuously: arping -I ethX xx.xx.xx.xx(self_ip) 3 Meanwhile, the guest also receives ARP packets continuously: tcpdump -i ethX arp host xx.xx.xx.xx(self_ip) -en 4 After migrateion, at the dest side, the guest gets a lot of ARP packets which came from the source-side guest(which was stored while it's suspended.). For example: 2015-03-27 16:45:56.695166 52:54:00:7d:b0:af ff:ff:ff:ff:ff:ff, ethertype ARP (0x0806), length 60: arp who-has 9.61.108.208 (ff:ff:ff:ff:ff:ff) tell 9.61.108.208 2015-03-27 16:45:56.695197 52:54:00:7d:b0:af ff:ff:ff:ff:ff:ff, ethertype ARP (0x0806), length 60: arp who-has 9.61.108.208 (ff:ff:ff:ff:ff:ff) tell 9.61.108.208 2015-03-27 16:45:56.695205 52:54:00:7d:b0:af ff:ff:ff:ff:ff:ff, ethertype ARP (0x0806), length 60: arp who-has 9.61.108.208 (ff:ff:ff:ff:ff:ff) tell 9.61.108.208 2015-03-27 16:45:56.695214 52:54:00:7d:b0:af ff:ff:ff:ff:ff:ff, ethertype ARP (0x0806), length 60: arp who-has 9.61.108.208 (ff:ff:ff:ff:ff:ff) tell 9.61.108.208 2015-03-27 16:45:56.695244 52:54:00:7d:b0:af ff:ff:ff:ff:ff:ff, ethertype ARP (0x0806), length 60: arp who-has 9.61.108.208 (ff:ff:ff:ff:ff:ff) tell 9.61.108.208 2015-03-27 16:45:56.695256 52:54:00:7d:b0:af ff:ff:ff:ff:ff:ff, ethertype ARP (0x0806), length 60: arp who-has 9.61.108.208 (ff:ff:ff:ff:ff:ff) tell 9.61.108.208 2015-03-27 16:45:56.695264 52:54:00:7d:b0:af ff:ff:ff:ff:ff:ff, ethertype ARP (0x0806), length 60: arp who-has 9.61.108.208 (ff:ff:ff:ff:ff:ff) tell 9.61.108.208 2015-03-27 16:45:56.695291 52:54:00:7d:b0:af ff:ff:ff:ff:ff:ff, ethertype ARP (0x0806), length 60: arp who-has 9.61.108.208 (ff:ff:ff:ff:ff:ff) tell 9.61.108.208 2015-03-27 16:45:56.695324 52:54:00:7d:b0:af ff:ff:ff:ff:ff:ff, ethertype ARP (0x0806), length 60: arp who-has 9.61.108.208 (ff:ff:ff:ff:ff:ff) tell 9.61.108.208 2015-03-27 16:45:56.695337 52:54:00:7d:b0:af ff:ff:ff:ff:ff:ff, ethertype ARP (0x0806), length 60: arp who-has 9.61.108.208 (ff:ff:ff:ff:ff:ff) tell 9.61.108.208 2015-03-27 16:45:56.695344 52:54:00:7d:b0:af ff:ff:ff:ff:ff:ff, ethertype ARP (0x0806), length 60: arp who-has 9.61.108.208 (ff:ff:ff:ff:ff:ff) tell 9.61.108.208 2015-03-27 16:45:56.695364 52:54:00:7d:b0:af ff:ff:ff:ff:ff:ff, ethertype ARP (0x0806), length 60: arp who-has 9.61.108.208 (ff:ff:ff:ff:ff:ff) tell 9.61.108.208 5 These packets will confuse my process. It may think that there is another VM has the same IP with itself. Reasons for the problem: The tap device will get up as soon as it's created(in virNetDevTapCreateInBridgePort), before the cpus got un-paused. So, it kept receiving data before the guest starts to run, please note that the data are sent from the source side. As soon as the guest get running, it parses the data stored before, and thinks they were from other guest with the same IP, which is in fact the guest from the source side. There was a patch network: Bring netdevs online later, it move the virNetDevSetOnline() of network device just before start VM's vcpu. But in the Laine Stump replay mail say It turns out, though, that regular tap devices which will be connected to a bridge should be ifup'ed and attached to the bridge as soon as possible, so that the forwarding delay timer of the bridge can start to count down. I agree with Laine Stump's idea that it's not a perfect solution to start the tap device right before running vcpu. so, here comes the question: what can we do to insure our guests not receive itself's ARP packets from src side during migrateion? -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH] qemu: lifecycle: make agent-mode shutdown and reboot timeout
When we shutdown/reboot a guest using agent-mode, if the guest itself blocks
infinitely,
libvirt would block in qemuAgentShutdown() forever.
Thus, we set a timeout for shutdown/reboot, from our experience, 60 seconds
would be fine.
Signed-off-by: Zhang Bo oscar.zhan...@huawei.com
Signed-off-by: Wang Yufei james.wangyu...@huawei.com
---
include/libvirt/libvirt-qemu.h | 1 +
src/qemu/qemu_agent.c | 2 +-
2 files changed, 2 insertions(+), 1 deletion(-)
diff --git a/include/libvirt/libvirt-qemu.h b/include/libvirt/libvirt-qemu.h
index 0c5d650..2bb8ee8 100644
--- a/include/libvirt/libvirt-qemu.h
+++ b/include/libvirt/libvirt-qemu.h
@@ -49,6 +49,7 @@ typedef enum {
VIR_DOMAIN_QEMU_AGENT_COMMAND_BLOCK = -2,
VIR_DOMAIN_QEMU_AGENT_COMMAND_DEFAULT = -1,
VIR_DOMAIN_QEMU_AGENT_COMMAND_NOWAIT = 0,
+VIR_DOMAIN_QEMU_AGENT_COMMAND_SHUTDOWN = 60,
} virDomainQemuAgentCommandTimeoutValues;
char *virDomainQemuAgentCommand(virDomainPtr domain, const char *cmd,
diff --git a/src/qemu/qemu_agent.c b/src/qemu/qemu_agent.c
index a7b3279..548d580 100644
--- a/src/qemu/qemu_agent.c
+++ b/src/qemu/qemu_agent.c
@@ -1300,7 +1300,7 @@ int qemuAgentShutdown(qemuAgentPtr mon,
else
mon-await_event = QEMU_AGENT_EVENT_SHUTDOWN;
ret = qemuAgentCommand(mon, cmd, reply, false,
- VIR_DOMAIN_QEMU_AGENT_COMMAND_BLOCK);
+ VIR_DOMAIN_QEMU_AGENT_COMMAND_SHUTDOWN);
virJSONValueFree(cmd);
virJSONValueFree(reply);
--
1.7.12.4
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [openstack-dev] [nova] The risk of hanging when shutdown instance.
On 2015/3/28 18:06, Rui Chen wrote: Thank you for reply, Chris. 2015-03-27 23:15 GMT+08:00 Chris Friesen chris.frie...@windriver.com mailto:chris.frie...@windriver.com: On 03/26/2015 07:44 PM, Rui Chen wrote: Yes, you are right, but we found our instance hang at first dom.shutdown() call, if the dom.shutdown() don't return, there is no chance to execute dom.destroy(), right? Correct. The code is written assuming dom.shutdown() cannot block indefinitely. The libvirt docs at https://libvirt.org/html/__libvirt-libvirt-domain.html#__virDomainShutdown https://libvirt.org/html/libvirt-libvirt-domain.html#virDomainShutdown say ...this command returns as soon as the shutdown request is issued rather than blocking until the guest is no longer running. If dom.shutdown() blocks indefinitely, then that's a libvirt bug. Chris The API virDomainShutdown's description is out of date, it's not correct. In fact, virDomainShutdown would block or not, depending on its mode. If it's in mode *agent*, then it would be blocked until qemu founds that the guest actually got down. Otherwise, if it's in mode *acpi*, then it would return immediately. Thus, maybe further more work need to be done in Openstack. What's your opinions, Michal and Daniel (from libvirt.org), and Chris (from openstack.org) :) -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] connect: ssh: Shall we remove the dependency of netcat ?
On 2015/3/28 0:25, Peter Krempa wrote: On Fri, Mar 27, 2015 at 10:54:26 +0800, zhang bo wrote: Too powerful? That is a ridiculous statement that probably originates from some kind of misunderstanding when creating a security policy or stuff like that. If a policy bans nc as powerful then it's missing on a lot of other options how to create listening or outgoing connections on arbitrary sockets. The only insecure part is that it does not use encryption, but that's a widely known fact about nc. Sorry for replying so late:) I tried to confirm the security fact the other days, unfortunately no clear answer gotten. What I meant was that the *network monitoring tools*, such as 'nc' and 'tcpdump', they are considered as dangerous for network security. They are prohibited in our company and some other organizations. I'm not quite sure whether the result that they're prohibited are directly related to their capability of monitoring network. But they actually got prohibited anyway. 3 So, is there any good substitution for netcat to realize qemu+ssh? Currently libvirt doesn't allow this, but I'm planning for a long time to introduce a standalone libvirt client binary (or perhaps add this as a mode to virsh) to replace the use of NC but that's due to other reasons: 1) nc doesn't know where session mode sockets are placed This is due to the fact that it depends on how libvirt is compiled. Currently the client side has to provide the path that is used at the remote side and those may not correspond. 2) errors reported when using the ssh connection transport are not helpful: NC is inherently bad at reporting what happened with the unix socket on the remote side. 3) getting rid of nc as a dependency This won't happen though ... old libvirt clients would not be able to connect to newer servers. In other words. I don't think libvirt will ever get rid of using nc but we can make stuff better by adding the internal remote client. Peter Thank you for the detailed reply, glad to hear that it's on schedule to be replaced someday. -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [openstack-dev] [nova] The risk of hanging when shutdown instance.
On 2015/3/31 4:36, Eric Blake wrote: On 03/30/2015 06:08 AM, Michal Privoznik wrote: On 30.03.2015 11:28, zhang bo wrote: On 2015/3/28 18:06, Rui Chen wrote: snip/ The API virDomainShutdown's description is out of date, it's not correct. In fact, virDomainShutdown would block or not, depending on its mode. If it's in mode *agent*, then it would be blocked until qemu founds that the guest actually got down. Otherwise, if it's in mode *acpi*, then it would return immediately. Thus, maybe further more work need to be done in Openstack. What's your opinions, Michal and Daniel (from libvirt.org), and Chris (from openstack.org) :) Yep, the documentation could be better in that respect. I've proposed a patch on the libvirt upstream list: https://www.redhat.com/archives/libvir-list/2015-March/msg01533.html I don't think a doc patch is right. If you don't pass any flags, then it is up to the hypervisor which method it will attempt (agent or ACPI). Yes, explicitly requesting an agent as the only method to attempt might be justifiable as a reason to block, but the overall API contract is to NOT block indefinitely. I think that rather than a doc patch, we need to fix the underlying bug, and guarantee that we return after a finite time even when the agent is involved. So, may we get to a final decision? :) Shall we timeout in virDomainShutdown() or leave it to openstack? The 2 solutions I can see are: 1) timeout in virDomainShutdown() and virDomainReboot(). in libvirt. 2) spawn a new thread to monitor the guest's status, if it's not shutoff after dom.shutdown() for a while, call dom.destroy() to force shut it down. in openstack. -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] connect: ssh: Shall we remove the dependency of netcat ?
1 When we connect libvirt with URI qemu+ssh, it uses 'nc' command to connect to libvirt-sock. # virsh -c qemu+ssh://root@9.61.1.74/system list Password: //ask users to input passwords here. IdName State 11pxerunning It in fact uses 'ssh' and 'nc' commands to connect to remote libvirt-sock, such as: ssh -l root 9.61.1.74 sh -c ''nc' -U /var/run/libvirt/libvirt-sock' The code path is : virConnectOpen-doRemoteOpen-virNetClientNewSSH-virNetSocketNewConnectSSH 2 However, netcat(nc) is considered as an insecure tool, because it's too powerful in controlling the network. It's abandoned by some organizations. 3 So, is there any good substitution for netcat to realize qemu+ssh? -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] lifecycle: shutdown: should it have timeout when using qemu-agent?
The problem we encountered is: 1) use qemu-agent to shutdown a domain. 2) libvirt will block in qemuAgentShutdown(), if the domain itself got stucked when it powers-off. It's the *guest domain*'s fault that it stucks when shutdown. However, we could not handle the domain in libvirt anymore, except for the jobs such as DESTROY or QUERY. So, here comes the question: What's the normal solution to this problem? shall we: 1) use DESTROY to forcelly poweroff the domain, in openstack or other applications that uses libvirt. or 2) leave it to the administrator users, if they find this problem, let them manually DESTROY the domain. or 3) let SHUTDOWN timeout in libvirt when the guest domain got stucked. Looking forward to your reply, thanks in advance. -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v2] util: don't fail if no PortData is found while getting migrateData
Thanks for your reply, this patch has already been commited by John on Mar.14, the commit ID is: 25df57db73adc3e610193ee1fcdd202c47ba471d Thank you all the same:) On 2015/3/21 0:10, Michal Privoznik wrote: On 04.03.2015 03:59, zhang bo wrote: I've tried to review this patch. However it is corrupt. --- V1 here: https://www.redhat.com/archives/libvir-list/2015-February/msg00388.html V2: Add --if-exists option to ovs-vsctl cmd, making ovs-vsctl not raise error if there's no portData available. Suggested by Martin. We Tested the patch, it works. --- I guess this ^^ is the problem. We put notes ... Introduced by f6a2f97e Problem description: After multiple times of migrating a domain, which has an ovs interface with no portData set, with non-shared disk, nbd ports got overflowed. The steps to reproduce the problem: 1 define and start a domain with its network configured as: interface type='bridge' source bridge='br0'/ virtualport type='openvswitch' /virtualport model type='virtio'/ driver name='vhost' queues='4'/ /interface 2 do not set the network's portData. 3 migrate(ToURI2) it with flag 91(1011011), which means: VIR_MIGRATE_LIVE VIR_MIGRATE_PEER2PEER VIR_MIGRATE_PERSIST_DEST VIR_MIGRATE_UNDEFINE_SOURCE VIR_MIGRATE_NON_SHARED_DISK 4 migrate success, but we got an error log in libvirtd.log: error : virCommandWait:2423 : internal error: Child process (ovs-vsctl --timeout=5 get Interface vnet1 external_ids:PortData) unexpected exit status 1: ovs-vsctl: no key PortData in Interface record vnet1 column external_ids 5 migrate it back, migrate it , migrate it back, ... 6 nbd port got overflowed. The reasons for the problem is : 1 virNetDevOpenvswitchGetMigrateData() takes it as wrong if no portData is available for the ovs interface of a domain. (We think it's not appropriate, as portData is just OPTIONAL) 2 in func qemuMigrationBakeCookie(), it fails in qemuMigrationCookieAddNetwork(), and returns with -1. qemuMigrationCookieAddNBD() is not called thereafter, and mig-nbd is still NULL. 3 However, qemuMigrationRun() just *WARN* if qemuMigrationBakeCookie() fails, migration still successes. cookie is NULL, it's not baked on the src side. 4 On the destination side, it would alloc a port first and then free the nbd port in COOKIE. But the cookie is NULL due to qemuMigrationCookieAddNetwork() failure at src side. thus the nbd port is not freed. In this patch, we add --if-exists option to make ovs-vsctl not raise error if there's no portData available. Further more, because portData may be NULL in the cookie at the dest side, check it before setting portData. Signed-off-by: Zhou Yimin zhouyi...@huawei.com Signed-off-by: Zhang Bo oscar.zhan...@huawei.com --- ... here. Afte this line. However, removing the block did not help. But editing a patch directly has very low success rate anyway. Can you please resend? src/util/virnetdevopenvswitch.c | 11 ++- 1 file changed, 10 insertions(+), 1 deletion(-) Michal . -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] util: vhost user: support for bootindex
On 2015/3/13 17:17, zhang bo wrote: Problem Description: When we set boot order for a vhost-user network interface, we found the boot index doesn't work. Cause of the Problem: In the function qemuBuildVhostuserCommandLine(), it forcely set the arg bootindex of function qemuBuildNicDevStr() to 0. Thus, the bootindex parameter got missing. Solution: Trans the arg bootindex down. Signed-off-by: Gao Haifeng gaohaifeng@huawei.com Signed-off-by: Zhang Bo oscar.zhan...@huawei.com .. ping ... -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH] util: vhost user: support for bootindex
Problem Description:
When we set boot order for a vhost-user network interface, we found the boot
index
doesn't work.
Cause of the Problem:
In the function qemuBuildVhostuserCommandLine(), it forcely set the arg
bootindex of
function qemuBuildNicDevStr() to 0. Thus, the bootindex parameter got missing.
Solution:
Trans the arg bootindex down.
Signed-off-by: Gao Haifeng gaohaifeng@huawei.com
Signed-off-by: Zhang Bo oscar.zhan...@huawei.com
---
src/qemu/qemu_command.c | 14 --
1 file changed, 8 insertions(+), 6 deletions(-)
diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
index 5303de5..2f37812 100644
--- a/src/qemu/qemu_command.c
+++ b/src/qemu/qemu_command.c
@@ -7767,7 +7767,8 @@ static int
qemuBuildVhostuserCommandLine(virCommandPtr cmd,
virDomainDefPtr def,
virDomainNetDefPtr net,
- virQEMUCapsPtr qemuCaps)
+ virQEMUCapsPtr qemuCaps,
+ int bootindex)
{
virBuffer chardev_buf = VIR_BUFFER_INITIALIZER;
virBuffer netdev_buf = VIR_BUFFER_INITIALIZER;
@@ -7814,7 +7815,7 @@ qemuBuildVhostuserCommandLine(virCommandPtr cmd,
virCommandAddArg(cmd, -netdev);
virCommandAddArgBuffer(cmd, netdev_buf);
-if (!(nic = qemuBuildNicDevStr(def, net, -1, 0, 0, qemuCaps))) {
+if (!(nic = qemuBuildNicDevStr(def, net, -1, bootindex, 0, qemuCaps))) {
virReportError(VIR_ERR_INTERNAL_ERROR,
%s, _(Error generating NIC -device string));
goto error;
@@ -7859,8 +7860,12 @@ qemuBuildInterfaceCommandLine(virCommandPtr cmd,
virNetDevBandwidthPtr actualBandwidth;
size_t i;
+
+if (!bootindex)
+bootindex = net-info.bootIndex;
+
if (actualType == VIR_DOMAIN_NET_TYPE_VHOSTUSER)
-return qemuBuildVhostuserCommandLine(cmd, def, net, qemuCaps);
+return qemuBuildVhostuserCommandLine(cmd, def, net, qemuCaps,
bootindex);
if (actualType == VIR_DOMAIN_NET_TYPE_HOSTDEV) {
/* NET_TYPE_HOSTDEV devices are really hostdev devices, so
@@ -7869,9 +7874,6 @@ qemuBuildInterfaceCommandLine(virCommandPtr cmd,
return 0;
}
-if (!bootindex)
-bootindex = net-info.bootIndex;
-
/* Currently nothing besides TAP devices supports multiqueue. */
if (net-driver.virtio.queues 0
!(actualType == VIR_DOMAIN_NET_TYPE_NETWORK ||
--
1.7.12.4
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] Should logging in the library set its default outputs?
If the env LIBVIRT_DEBUG and LIBVIRT_LOG_OUTPUTS are not set, the logging in the library would not log into any file. So: Is it necessary to set the default level and outputs in virGlobalInit(), just in case the maintainer forgets to set the ENVs ? Thanks in advance. -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] logging: how about adding a ProcessName field in logging file?
Suppose there are 3 or more clients of libvirt: 1)nova 2)bash virsh commands 3)user customized ELF 4)etc The env LIBVIRT_DEBUG and LIBVIRT_LOG_OUTPUTS affects all of these clients, thus, they will all accumulate the logs into *ONE* file set by LIBVIRT_LOG_OUTPUTS. eg: [2015-03-07 00:33:30]: 103674: info : virDomainShutdown:3242 : enter virDomainShutdown domainname=VMName [2015-03-07 00:33:41]: 103674: info : virDomainShutdown:3253 : domain VMName shutted down [2015-03-13 00:53:44]: 5073: info : libvirt version: 1.2.7 [2015-03-13 00:53:44]: 5034: info : libvirt version: 1.2.7 [2015-03-13 00:53:44]: 5073: error : virNetSocketReadWire:1475 : End of file while reading data: Input/output error note: 103674: bash virsh command 5037: nova if we don't know that 103674 is just a virsh command, and suspect that it's nova, time would be wasted to find out who's the criminal. The improved log would be: [2015-03-07 00:33:30]: virsh: 103674: info : virDomainShutdown:3242 : enter virDomainShutdown domainname=VMName [2015-03-07 00:33:41]: virsh: 103674: info : virDomainShutdown:3253 : domain VMName shutted down [2015-03-13 00:53:44]: nova: 5073: info : libvirt version: 1.2.7 [2015-03-13 00:53:44]: myProc1: 5034: info : libvirt version: 1.2.7 [2015-03-13 00:53:44]: nova: error : virNetSocketReadWire:1475 : End of file while reading data: Input/output error So, here's the qeustion: Is it neccssary to add a ProcessName field in the logging file? if so, I'd like to apply a patch for this. thank you in advance. -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] logging: how about adding a ProcessName field in logging file?
On 2015/3/13 17:29, Daniel P. Berrange wrote: On Fri, Mar 13, 2015 at 05:08:54PM +0800, zhang bo wrote: Suppose there are 3 or more clients of libvirt: 1)nova 2)bash virsh commands 3)user customized ELF 4)etc The env LIBVIRT_DEBUG and LIBVIRT_LOG_OUTPUTS affects all of these clients, thus, they will all accumulate the logs into *ONE* file set by LIBVIRT_LOG_OUTPUTS. There is no attempt to make sure that separate clients logging to the same file will atomically write log lines. You could get half a line of text from one client, then half a line of text from a second client, then the rest of the line from the first client all mangled up. You simply shouldn't give each client process the same logging output file. Regards, Daniel Got it, thank you for your immediate reply :) -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH v3] util: don't fail if no PortData is found while getting migrateData
Introduced by f6a2f97e
Problem Description:
After multiple times of migrating a domain, which has an ovs interface with no
portData set,
with non-shared disk, nbd ports got overflowed.
The steps to reproduce the problem:
1 define and start a domain with its network configured as:
interface type='bridge'
source bridge='br0'/
virtualport type='openvswitch'
/virtualport
model type='virtio'/
driver name='vhost' queues='4'/
/interface
2 do not set the network's portData.
3 migrate(ToURI2) it with flag 91(1011011), which means:
VIR_MIGRATE_LIVE
VIR_MIGRATE_PEER2PEER
VIR_MIGRATE_PERSIST_DEST
VIR_MIGRATE_UNDEFINE_SOURCE
VIR_MIGRATE_NON_SHARED_DISK
4 migrate success, but we got an error log in libvirtd.log:
error : virCommandWait:2423 : internal error: Child process (ovs-vsctl
--timeout=5 get Interface
vnet1 external_ids:PortData) unexpected exit status 1: ovs-vsctl: no key
PortData in Interface
record vnet1 column external_ids
5 migrate it back, migrate it , migrate it back, ...
6 nbd port got overflowed.
The reasons for the problem is :
1 virNetDevOpenvswitchGetMigrateData() takes it as wrong if no portData is
available for the ovs
interface of a domain. (We think it's not appropriate, as portData is just
OPTIONAL)
2 in func qemuMigrationBakeCookie(), it fails in
qemuMigrationCookieAddNetwork(), and returns with -1.
qemuMigrationCookieAddNBD() is not called thereafter, and mig-nbd is still
NULL.
3 However, qemuMigrationRun() just *WARN* if qemuMigrationBakeCookie() fails,
migration still successes.
cookie is NULL, it's not baked on the src side.
4 On the destination side, it would alloc a port first and then free the nbd
port in COOKIE.
But the cookie is NULL due to qemuMigrationCookieAddNetwork() failure at src
side. thus the nbd port
is not freed.
In this patch, we add --if-exists option to make ovs-vsctl not raise error if
there's no portData available.
Further more, because portData may be NULL in the cookie at the dest side,
check it before setting portData.
Signed-off-by: Zhou Yimin zhouyi...@huawei.com
Signed-off-by: Zhang Bo oscar.zhan...@huawei.com
---
V1 here:
https://www.redhat.com/archives/libvir-list/2015-February/msg00388.html
V2:
Add --if-exists option to ovs-vsctl cmd, making ovs-vsctl not raise error
if there's no portData
available. Suggested by Martin.
We Tested the patch, it works.
link: https://www.redhat.com/archives/libvir-list/2015-March/msg00104.html
V3:
move V1/V2/V3 description from above the patch to here. It did not show the
detailed description above
after apply PATCH v2.
---
src/util/virnetdevopenvswitch.c | 11 ++-
1 file changed, 10 insertions(+), 1 deletion(-)
diff --git a/src/util/virnetdevopenvswitch.c b/src/util/virnetdevopenvswitch.c
index e5c87bb..722d0dd 100644
--- a/src/util/virnetdevopenvswitch.c
+++ b/src/util/virnetdevopenvswitch.c
@@ -30,9 +30,12 @@
#include virerror.h
#include virmacaddr.h
#include virstring.h
+#include virlog.h
#define VIR_FROM_THIS VIR_FROM_NONE
+VIR_LOG_INIT(util.netdevopenvswitch);
+
/**
* virNetDevOpenvswitchAddPort:
* @brname: the bridge name
@@ -206,7 +209,7 @@ int virNetDevOpenvswitchGetMigrateData(char **migrate,
const char *ifname)
virCommandPtr cmd = NULL;
int ret = -1;
-cmd = virCommandNewArgList(OVSVSCTL, --timeout=5, get, Interface,
+cmd = virCommandNewArgList(OVSVSCTL, --timeout=5, --if-exists, get,
Interface,
ifname, external_ids:PortData, NULL);
virCommandSetOutputBuffer(cmd, migrate);
@@ -241,6 +244,12 @@ int virNetDevOpenvswitchSetMigrateData(char *migrate,
const char *ifname)
virCommandPtr cmd = NULL;
int ret = -1;
+if (!migrate) {
+VIR_DEBUG(No OVS port data for interface %s, ifname);
+return 0;
+}
+
cmd = virCommandNewArgList(OVSVSCTL, --timeout=5, set,
Interface, ifname, NULL);
virCommandAddArgFormat(cmd, external_ids:PortData=%s, migrate);
--
1.7.12.4
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH v2] util: don't fail if no PortData is found while getting migrateData
---
V1 here:
https://www.redhat.com/archives/libvir-list/2015-February/msg00388.html
V2:
Add --if-exists option to ovs-vsctl cmd, making ovs-vsctl not raise error
if there's no portData
available. Suggested by Martin.
We Tested the patch, it works.
---
Introduced by f6a2f97e
Problem description:
After multiple times of migrating a domain, which has an ovs interface with no
portData set,
with non-shared disk, nbd ports got overflowed.
The steps to reproduce the problem:
1 define and start a domain with its network configured as:
interface type='bridge'
source bridge='br0'/
virtualport type='openvswitch'
/virtualport
model type='virtio'/
driver name='vhost' queues='4'/
/interface
2 do not set the network's portData.
3 migrate(ToURI2) it with flag 91(1011011), which means:
VIR_MIGRATE_LIVE
VIR_MIGRATE_PEER2PEER
VIR_MIGRATE_PERSIST_DEST
VIR_MIGRATE_UNDEFINE_SOURCE
VIR_MIGRATE_NON_SHARED_DISK
4 migrate success, but we got an error log in libvirtd.log:
error : virCommandWait:2423 : internal error: Child process (ovs-vsctl
--timeout=5 get Interface
vnet1 external_ids:PortData) unexpected exit status 1: ovs-vsctl: no key
PortData in Interface
record vnet1 column external_ids
5 migrate it back, migrate it , migrate it back, ...
6 nbd port got overflowed.
The reasons for the problem is :
1 virNetDevOpenvswitchGetMigrateData() takes it as wrong if no portData is
available for the ovs
interface of a domain. (We think it's not appropriate, as portData is just
OPTIONAL)
2 in func qemuMigrationBakeCookie(), it fails in
qemuMigrationCookieAddNetwork(), and returns with -1.
qemuMigrationCookieAddNBD() is not called thereafter, and mig-nbd is still
NULL.
3 However, qemuMigrationRun() just *WARN* if qemuMigrationBakeCookie() fails,
migration still successes.
cookie is NULL, it's not baked on the src side.
4 On the destination side, it would alloc a port first and then free the nbd
port in COOKIE.
But the cookie is NULL due to qemuMigrationCookieAddNetwork() failure at src
side. thus the nbd port
is not freed.
In this patch, we add --if-exists option to make ovs-vsctl not raise error if
there's no portData available.
Further more, because portData may be NULL in the cookie at the dest side,
check it before setting portData.
Signed-off-by: Zhou Yimin zhouyi...@huawei.com
Signed-off-by: Zhang Bo oscar.zhan...@huawei.com
---
src/util/virnetdevopenvswitch.c | 11 ++-
1 file changed, 10 insertions(+), 1 deletion(-)
diff --git a/src/util/virnetdevopenvswitch.c b/src/util/virnetdevopenvswitch.c
index e5c87bb..722d0dd 100644
--- a/src/util/virnetdevopenvswitch.c
+++ b/src/util/virnetdevopenvswitch.c
@@ -30,9 +30,12 @@
#include virerror.h
#include virmacaddr.h
#include virstring.h
+#include virlog.h
#define VIR_FROM_THIS VIR_FROM_NONE
+VIR_LOG_INIT(util.netdevopenvswitch);
+
/**
* virNetDevOpenvswitchAddPort:
* @brname: the bridge name
@@ -206,7 +209,7 @@ int virNetDevOpenvswitchGetMigrateData(char **migrate,
const char *ifname)
virCommandPtr cmd = NULL;
int ret = -1;
-cmd = virCommandNewArgList(OVSVSCTL, --timeout=5, get, Interface,
+cmd = virCommandNewArgList(OVSVSCTL, --timeout=5, --if-exists, get,
Interface,
ifname, external_ids:PortData, NULL);
virCommandSetOutputBuffer(cmd, migrate);
@@ -241,6 +244,12 @@ int virNetDevOpenvswitchSetMigrateData(char *migrate,
const char *ifname)
virCommandPtr cmd = NULL;
int ret = -1;
+if (!migrate) {
+VIR_DEBUG(No port data for interface %s, ifname);
+return 0;
+}
+
cmd = virCommandNewArgList(OVSVSCTL, --timeout=5, set,
Interface, ifname, NULL);
virCommandAddArgFormat(cmd, external_ids:PortData=%s, migrate);
--
1.7.12.4
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] util: do not take it as wrong if no PortData is found while getting migrateData
On 2015/2/20 18:05, Martin Kletzander wrote:
On Thu, Feb 12, 2015 at 12:08:54PM +0800, Zhang Bo wrote:
The function virNetDevOpenvswitchGetMigrateData() uses the cmd ovs-vsctl to
get portdata. If no portdata is available, rather than failure in running
the cmd, we think we should just print a warning message here, rather than
taking it as wrong, because PortData is just optional for an ovs interface.
If we take it as wrong anyway, in function qemuMigrationBakeCookie() it will
terminate in the middle, and cookies such as NBD would not be baked, further
more errors would happen, such as nbd ports get overflowed, etc.
Thus, we add an if branch in virNetDevOpenvswitchGetMigrateData() to just
warn
if portdata is unavailable.
Sorry for replying so late, we were having Spring-Festival holiday then:)
Won't that data be missing then?
Yes, the portdata will be missing, but as that it's *OPTIONAL*, missing is not
a big
deal.
We can tell that it's optional from the comments in
qemuMigrationCookieNetworkXMLParse():
port data is optional, and may not exist.
It's optional as well as other external_ids for an ovs interface.
I would format the subject line as don't fail if... to make it brief
and clean, but that's just a nit. Anyway, I see multiple problems
with this patch.
I would modify that if this patch is decided to be acceptable later.
But first let me ask a question; how come there are no PortData on the
destination, when we set them unconditionally? I mean how did you get
to this problem in the first place?
Sorry for missing the detailed description of the original problem.
The problem was actually:
After multiple times of migrating a domain, which has an ovs interface
with no portData set, with non-shared disk, nbd ports got overflowed.
The steps to reproduce the problem:
1 define and start a domain with its network configured as:
interface type='bridge'
source bridge='br0'/
virtualport type='openvswitch'
/virtualport
model type='virtio'/
driver name='vhost' queues='4'/
/interface
2 do not set the network's portData.
3 migrate(ToURI2) it with flag 91(1011011), which means:
VIR_MIGRATE_LIVE
VIR_MIGRATE_PEER2PEER
VIR_MIGRATE_PERSIST_DEST
VIR_MIGRATE_UNDEFINE_SOURCE
VIR_MIGRATE_NON_SHARED_DISK
4 migrate success, but we got an error log in libvirtd.log:
error : virCommandWait:2423 : internal error: Child process (ovs-vsctl
--timeout=5
get Interface vnet1 external_ids:PortData) unexpected exit status 1: ovs-vsctl:
no
key PortData in Interface record vnet1 column external_ids
5 migrate it back, migrate it , migrate it back, ...
6 nbd port got overflowed.
The nbd port problem is directly caused by the problem in step 4. In fact the
patch here just fixed
problem 4. If we want to totally solve problem in step 6, lots more work has
to be done.
We'll study the nbd problem later. If you have any suggestion, please let me
know, thanks.
The reasons for the problem is :
1 virNetDevOpenvswitchGetMigrateData() takes it as wrong if no portData is
available for
the ovs interface of a domain. (We think it's not appropriate, as portData is
just OPTIONAL)
2 in func qemuMigrationBakeCookie(), it fails in
qemuMigrationCookieAddNetwork(), and returns
with -1. qemuMigrationCookieAddNBD() is not called thereafter, and mig-nbd is
still NULL.
3 However, qemuMigrationRun() just *WARN* if qemuMigrationBakeCookie() fails,
migration still successes.
cookie is NULL, it's not baked on the src side.
4 On the destination side, it would alloc a port first and then free the nbd
port in COOKIE.
But the cookie is NULL due to qemuMigrationCookieAddNetwork() failure at src
side. thus the nbd port
is not freed.
Signed-off-by: Zhang Bo oscar.zhan...@huawei.com
Signed-off-by: Zhou Yimin zhouyi...@huawei.com
---
src/util/virnetdevopenvswitch.c | 28
1 file changed, 24 insertions(+), 4 deletions(-)
diff --git a/src/util/virnetdevopenvswitch.c
b/src/util/virnetdevopenvswitch.c
index e5c87bb..6116377 100644
--- a/src/util/virnetdevopenvswitch.c
+++ b/src/util/virnetdevopenvswitch.c
@@ -30,9 +30,12 @@
#include virerror.h
#include virmacaddr.h
#include virstring.h
+#include virlog.h
#define VIR_FROM_THIS VIR_FROM_NONE
+VIR_LOG_INIT(util.netdevopenvswitch);
+
/**
* virNetDevOpenvswitchAddPort:
* @brname: the bridge name
@@ -204,18 +207,29 @@ int virNetDevOpenvswitchRemovePort(const char *brname
ATTRIBUTE_UNUSED, const ch
int virNetDevOpenvswitchGetMigrateData(char **migrate, const char *ifname)
{
virCommandPtr cmd = NULL;
+char *errbuf = NULL;
int ret = -1;
cmd = virCommandNewArgList(OVSVSCTL, --timeout=5, get, Interface,
ifname, external_ids:PortData, NULL);
virCommandSetOutputBuffer(cmd, migrate);
+virCommandSetErrorBuffer(cmd, errbuf);
/* Run the command */
-if (virCommandRun(cmd, NULL) 0
[libvirt] why not shutdown the src vm to avoid split-brain
In func doPeer2PeerMigrate3(), in the finish step, it checks whether domainMigrateFinish3() returns NULL or not. if it(ddomain) is NULL, it just restarts the guest on the source. Please consider the scenario that the ddomain has already been running on the dest, but it fails to tell the source this fact, and ddomain becomes NULL. If we then restart the guest on the source, there will be 2 same guests running on both sides, and a SPLIT-BRAIN occurs. It seems much better to stop them both , rather than leaving them both running. At least, when we found the ddomain is NULL, we should probably check whether the problem is caused by keepAlive failure, if so, kill the guest on the source rather than restarting it. How do you think about that? BTW, it says that: The lock manager plugins should take care of safety in this scenario in the comment, with the commit 2593f9692df0f128b14cde811e18aa49c1cf3e06, I don't quite understand that: 1) If we migrate the guest with the flag VIR_MIGRATE_NON_SHARED_DISK, then nbd server may take care of the data consistency, But before it starts the cpus on the dest, the nbd server is already stopped. So, at this moment, no one takes care of this problem. 2) If we migrate the guest with a shared disk, then does it mean that the nfs or other shareing-disk schemas should prevent split-brain by themselves? -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH] util: do not resotre the VF that is in use by another active guest
If we assign a VF, which has already been used by an active guest, to another
guest, and try to start the 2nd guest later on, the 2nd guest would not
start, and the VF won't work anymore.
Steps to reproduce the problem:
1 Assign a VF to guest A, and start the guest. The VF works fine.
2 Assign the VF to guest B, and try to start guest B. guest B can't start.
3 Guest A's network becomes unreachable, because its VF now doesn't work.
Reasons for this problem is:
1 When we start guest B, libvirtd checks whether the VF is already used
by another guest, if so, qemuPrepareHostDevices() returns with failure.
2 Then, libvirtd calls qemuProcessStop() to cleanup resourses, which would
restore the VFs of guest B. Specifically, it reads
/var/run/libvirt/hostdevmgr/ethX_vfX to get the VF's original MAC/VLAN,
and set it back to current VF.
3 As that the VF is still in use by guest A, libvirtd just set its MAC/VLAN
to another value, the VF doesn't work anymore.
Detailed flow:
qemuProcessStart
\___qemuPrepareHostDevices(if it fails, goto cleanup)
\ \_qemuPrepareHostdevPCIDevices
\\_virHostdevPreparePCIDevices
\ \LOOP1:virPCIDeviceListFind
\ (whether the device is in use by another
active guest)
\ if the VF has been assigned to,
qemuPrepareHostDevices() fails
\___cleanup:
\__qemuProcessStop
\qemuDomainReAttachHostDevices
\qemuDomainReAttachHostdevDevices
\virHostdevReAttachPCIDevices
\_virHostdevNetConfigRestore
(it gets MAC/VLAN form
/var/run/libvirt/hostdevmgr/ethX_vfX,
and set it back to the VF, making
the VF unusable)
This patch checks whether the VF is already in use before restoring it.
Signed-off-by: Zhang Bo oscar.zhan...@huawei.com
Signed-off-by: Zhuang Yanying zhuangyany...@huawei.com
---
src/util/virhostdev.c | 14 +-
1 file changed, 13 insertions(+), 1 deletion(-)
diff --git a/src/util/virhostdev.c b/src/util/virhostdev.c
index 9678e2b..ee19400 100644
--- a/src/util/virhostdev.c
+++ b/src/util/virhostdev.c
@@ -816,9 +816,21 @@ virHostdevReAttachPCIDevices(virHostdevManagerPtr
hostdev_mgr,
* For SRIOV net host devices, unset mac and port profile before
* reset and reattach device
*/
-for (i = 0; i nhostdevs; i++)
+for (i = 0; i nhostdevs; i++){
+ virPCIDevicePtr devNotInuse = NULL;
+ virDevicePCIAddressPtr addr = NULL;
+ virDomainHostdevDefPtr hostdev = hostdevs[i];
+ addr = hostdev-source.subsys.u.pci.addr;
+ devNotInuse = virPCIDeviceListFindByIDs(pcidevs,
+ addr-domain, addr-bus,
+ addr-slot, addr-function);
+ if (!devNotInuse) {
+ continue;
+ }
+
virHostdevNetConfigRestore(hostdevs[i], hostdev_mgr-stateDir,
oldStateDir);
+}
for (i = 0; i virPCIDeviceListCount(pcidevs); i++) {
virPCIDevicePtr dev = virPCIDeviceListGet(pcidevs, i);
--
1.7.12.4
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH] Fix a syntax error in the description text of libvirtd.conf
not yet not - not yet. Signed-off-by: Zhang Bo oscar.zhan...@huawei.com --- daemon/libvirtd.conf | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/daemon/libvirtd.conf b/daemon/libvirtd.conf index d4f6a1c..069ef3a 100644 --- a/daemon/libvirtd.conf +++ b/daemon/libvirtd.conf @@ -272,7 +272,7 @@ # connection succeeds. #max_queued_clients = 1000 -# The maximum length of queue of accepted but not yet not +# The maximum length of queue of accepted but not yet # authenticated clients. The default value is zero, meaning # the feature is disabled. #max_anonymous_clients = 20 -- 1.7.12.4 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH] conf: Fix libvirtd crash and memory leak caused by virDomainVcpuPinDel()
The function virDomainVcpuPinDel() used vcpupin_list to stand for
def-cputune.vcpupin, which made the codes more readable.
However, in this function, it will realloc vcpupin_list later.
As the definition of realloc(), it may free vcpupin_list and then
points it to a new-realloced address, but def-cputune.vcpupin doesn't
point to the new address(it's freed however).
Thus,
1) When we refer to the def-cputune.vcpupin afterwards, which was freed
by realloc(), an INVALID READ occurs, and libvirtd may crash.
2) As no one will use vcpupin_list any more, and no one frees it(it's just
alloced by realloc()), memory leak occurs.
Part of the valgrind logs are shown as below:
==1837== Thread 15:
==1837== Invalid read of size 8
==1837==at 0x5367337: virDomainDefFormatInternal (domain_conf.c:18392)
which is : virBufferAsprintf(buf, vcpupin vcpu='%u' ,
def-cputune.vcpupin[i]-vcpuid);
==1837==by 0x536966C: virDomainObjFormat (domain_conf.c:18970)
==1837==by 0x5369743: virDomainSaveStatus (domain_conf.c:19166)
==1837==by 0x117B26DC: qemuDomainPinVcpuFlags (qemu_driver.c:4586)
==1837==by 0x53EA313: virDomainPinVcpuFlags (libvirt.c:9803)
==1837==by 0x14CB7D: remoteDispatchDomainPinVcpuFlags
(remote_dispatch.h:6762)
==1837==by 0x14CC81: remoteDispatchDomainPinVcpuFlagsHelper
(remote_dispatch.h:6740)
==1837==by 0x5464C30: virNetServerProgramDispatchCall
(virnetserverprogram.c:437)
==1837==by 0x546507A: virNetServerProgramDispatch
(virnetserverprogram.c:307)
==1837==by 0x171B83: virNetServerProcessMsg (virnetserver.c:172)
==1837==by 0x171E6E: virNetServerHandleJob (virnetserver.c:193)
==1837==by 0x5318E78: virThreadPoolWorker (virthreadpool.c:145)
==1837== Address 0x12ea2870 is 0 bytes inside a block of size 16 free'd
==1837==at 0x4C291AC: realloc (in
/usr/lib64/valgrind/vgpreload_memcheck-amd64-linux.so)
==1837==by 0x52A3D14: virReallocN (viralloc.c:245)
==1837==by 0x52A3DFB: virShrinkN (viralloc.c:372)
==1837==by 0x52A3F57: virDeleteElementsN (viralloc.c:503)
==1837==by 0x533939E: virDomainVcpuPinDel (domain_conf.c:15405)
//doReset为true时才会进到。
==1837==by 0x117B2642: qemuDomainPinVcpuFlags (qemu_driver.c:4573)
==1837==by 0x53EA313: virDomainPinVcpuFlags (libvirt.c:9803)
==1837==by 0x14CB7D: remoteDispatchDomainPinVcpuFlags
(remote_dispatch.h:6762)
==1837==by 0x14CC81: remoteDispatchDomainPinVcpuFlagsHelper
(remote_dispatch.h:6740)
==1837==by 0x5464C30: virNetServerProgramDispatchCall
(virnetserverprogram.c:437)
==1837==by 0x546507A: virNetServerProgramDispatch
(virnetserverprogram.c:307)
==1837==by 0x171B83: virNetServerProcessMsg (virnetserver.c:172)
Steps to reproduce the problem:
1) use virDomainPinVcpuFlags() to pin a guest's vcpu to all the pcpus
of the host.
This patch uses def-cputune.vcpupin instead of vcpupin_list to do the
realloc() job, to avoid invalid read or memory leaking.
Signed-off-by: Zhang Bo oscar.zhan...@huawei.com
Signed-off-by: Yue Wenyuan yueweny...@huawei.com@huawei.com
---
src/conf/domain_conf.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index f0b715d..e1a3024 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -16276,7 +16276,7 @@ virDomainVcpuPinDel(virDomainDefPtr def, int vcpu)
if (vcpupin_list[n]-vcpuid == vcpu) {
virBitmapFree(vcpupin_list[n]-cpumask);
VIR_FREE(vcpupin_list[n]);
-VIR_DELETE_ELEMENT(vcpupin_list, n, def-cputune.nvcpupin);
+VIR_DELETE_ELEMENT(def-cputune.vcpupin, n, def-cputune.nvcpupin);
return;
}
}
--
1.7.12.4
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH] util: do not take it as wrong if no PortData is found while getting migrateData
The function virNetDevOpenvswitchGetMigrateData() uses the cmd ovs-vsctl to
get portdata. If no portdata is available, rather than failure in running
the cmd, we think we should just print a warning message here, rather than
taking it as wrong, because PortData is just optional for an ovs interface.
If we take it as wrong anyway, in function qemuMigrationBakeCookie() it will
terminate in the middle, and cookies such as NBD would not be baked, further
more errors would happen, such as nbd ports get overflowed, etc.
Thus, we add an if branch in virNetDevOpenvswitchGetMigrateData() to just warn
if portdata is unavailable.
Signed-off-by: Zhang Bo oscar.zhan...@huawei.com
Signed-off-by: Zhou Yimin zhouyi...@huawei.com
---
src/util/virnetdevopenvswitch.c | 28
1 file changed, 24 insertions(+), 4 deletions(-)
diff --git a/src/util/virnetdevopenvswitch.c b/src/util/virnetdevopenvswitch.c
index e5c87bb..6116377 100644
--- a/src/util/virnetdevopenvswitch.c
+++ b/src/util/virnetdevopenvswitch.c
@@ -30,9 +30,12 @@
#include virerror.h
#include virmacaddr.h
#include virstring.h
+#include virlog.h
#define VIR_FROM_THIS VIR_FROM_NONE
+VIR_LOG_INIT(util.netdevopenvswitch);
+
/**
* virNetDevOpenvswitchAddPort:
* @brname: the bridge name
@@ -204,18 +207,29 @@ int virNetDevOpenvswitchRemovePort(const char *brname
ATTRIBUTE_UNUSED, const ch
int virNetDevOpenvswitchGetMigrateData(char **migrate, const char *ifname)
{
virCommandPtr cmd = NULL;
+char *errbuf = NULL;
int ret = -1;
cmd = virCommandNewArgList(OVSVSCTL, --timeout=5, get, Interface,
ifname, external_ids:PortData, NULL);
virCommandSetOutputBuffer(cmd, migrate);
+virCommandSetErrorBuffer(cmd, errbuf);
/* Run the command */
-if (virCommandRun(cmd, NULL) 0) {
-virReportError(VIR_ERR_INTERNAL_ERROR,
- _(Unable to run command to get OVS port data for
- interface %s), ifname);
+ret = virCommandRun(cmd, NULL);
+if (ret 0) {
+/*PortData is optional, thus do not take it as wrong if the PortData
is not found.*/
+if (strstr(errbuf, no key \PortData\ in Interface record)) {
+VIR_WARN(Can't find OVS port data for interface %s, ifname);
+if (*migrate)
+VIR_FREE(*migrate);
+ret = 0;
+} else {
+virReportError(VIR_ERR_INTERNAL_ERROR,
+ _(Unable to run command to get OVS port data
for
+ interface %s), ifname);
+}
goto cleanup;
}
@@ -223,6 +237,7 @@ int virNetDevOpenvswitchGetMigrateData(char **migrate,
const char *ifname)
(*migrate)[strlen(*migrate) - 1] = '\0';
ret = 0;
cleanup:
+VIR_FREE(errbuf);
virCommandFree(cmd);
return ret;
}
@@ -241,6 +256,11 @@ int virNetDevOpenvswitchSetMigrateData(char *migrate,
const char *ifname)
virCommandPtr cmd = NULL;
int ret = -1;
+if (NULL == migrate) {
+VIR_INFO(There is no need to set OVS port data for interface %s,
ifname);
+return 0;
+}
+
cmd = virCommandNewArgList(OVSVSCTL, --timeout=5, set,
Interface, ifname, NULL);
virCommandAddArgFormat(cmd, external_ids:PortData=%s, migrate);
--
1.7.12.4
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] It does not support hot-plugging VHOST-USER networkcard in libvirt?
It works well if we use qmp command to directly interact with qemu 2.1 to hot-add vhost-user network card. However, libvirt seems not support hotplugging well. Steps to use qmp commands to directly(without libvirtd) interact with qemu 2.1 : 1 (qemu) chardev-add backend=socket,id=charnet1,path=/var/run/vhost-user/tap1 2 (qemu) netdev_add vhost-user,id=hostnet1,chardev=charnet1 3 (qemu) device_add virtio-net-pci,netdev=hostnet1,id=net1 In libvirt, we found that, Hot-plug is finished with a success result, but the network card doesn't work afterwards, it could not send/receive IOs. As we further digged into the problem, the 1st step of qmp commands is missing if we use libvirt to do the hot-plug job. 1 (qemu) chardev-add backend=socket,id=charnet1,path=/var/run/vhost-user/tap1 After looking into the function qemuDomainAttachNetDevice(), we found that it does have 1) qemuMonitorAddNetdev() 2) qemuMonitorAddDevice() except for *ChardevAdd*() The question is : why does qemuDomainAttachNetDevice() not do the *ChardevAdd*() work? It seems necessary to add chardev to add a vhost-user network card. -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
