[libvirt] [PATCH] qemu: Fix define logic
by Michal Privoznik
With current flow in qemudDomainDefine we might loose data
when updating an existing domain. We parse given XML and
overwrite the configuration. Then we try to save the new
config. However, this step may fail and we don't perform any
roll back. In fact, we remove the domain from the list of
domains held up by qemu driver. This is okay as long as the
domain was brand new one.
---
The easiest steps to reproduce:
https://bugzilla.redhat.com/show_bug.cgi?id=851963
src/qemu/qemu_driver.c | 43 ++++++++++++++++++++++++++++++++++++-------
1 files changed, 36 insertions(+), 7 deletions(-)
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index 3fcca0e..e760ed9 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -5538,6 +5538,7 @@ out:
static virDomainPtr qemudDomainDefine(virConnectPtr conn, const char *xml) {
struct qemud_driver *driver = conn->privateData;
virDomainDefPtr def;
+ virDomainDefPtr def_backup = NULL;
virDomainObjPtr vm = NULL;
virDomainPtr dom = NULL;
virDomainEventPtr event = NULL;
@@ -5561,20 +5562,48 @@ static virDomainPtr qemudDomainDefine(virConnectPtr conn, const char *xml) {
if (qemuDomainAssignAddresses(def, NULL, NULL) < 0)
goto cleanup;
- if (!(vm = virDomainAssignDef(driver->caps,
- &driver->domains,
- def, false))) {
- goto cleanup;
+ /* We need to differentiate two cases:
+ * a) updating an existing domain - must preserve previous definition
+ * so we can roll back if something fails
+ * b) defining a brand new domain - virDomainAssignDef is just sufficient
+ */
+ if ((vm = virDomainFindByUUID(&driver->domains, def->uuid))) {
+ if (virDomainObjIsActive(vm)) {
+ def_backup = vm->newDef;
+ vm->newDef = def;
+ } else {
+ def_backup = vm->def;
+ vm->def = def;
+ }
+ } else {
+ if (!(vm = virDomainAssignDef(driver->caps,
+ &driver->domains,
+ def, false))) {
+ goto cleanup;
+ }
}
def = NULL;
vm->persistent = 1;
if (virDomainSaveConfig(driver->configDir,
vm->newDef ? vm->newDef : vm->def) < 0) {
- VIR_INFO("Defining domain '%s'", vm->def->name);
- qemuDomainRemoveInactive(driver, vm);
- vm = NULL;
+ if (def_backup) {
+ /* There is backup so this VM was defined before.
+ * Just restore the backup. */
+ VIR_INFO("Restoring domain '%s' definition", vm->def->name);
+ if (virDomainObjIsActive(vm))
+ vm->newDef = def_backup;
+ else
+ vm->def = def_backup;
+ } else {
+ /* Brand new domain. Remove it */
+ VIR_INFO("Deleting domain '%s'", vm->def->name);
+ qemuDomainRemoveInactive(driver, vm);
+ vm = NULL;
+ }
goto cleanup;
+ } else {
+ virDomainDefFree(def_backup);
}
event = virDomainEventNewFromObj(vm,
--
1.7.8.6
12 years, 2 months
[libvirt] [PATCH 1/2] Introduce new VIR_ERR_AGENT_UNRESPONSIVE error code
by Michal Privoznik
Currently, when guest agent is configure but not responsive
(e.g. due to appropriate service not running in the guest)
we return VIR_ERR_INTERNAL_ERROR or VIR_ERR_ARGUMENT_UNSUPPORTED.
Both are wrong. Therefore we need to introduce new error code
to reflect this case.
---
include/libvirt/virterror.h | 2 ++
src/qemu/qemu_agent.c | 2 +-
src/qemu/qemu_driver.c | 24 ++++++++++++++----------
src/util/virterror.c | 7 +++++++
4 files changed, 24 insertions(+), 11 deletions(-)
diff --git a/include/libvirt/virterror.h b/include/libvirt/virterror.h
index 69c64aa..5140c38 100644
--- a/include/libvirt/virterror.h
+++ b/include/libvirt/virterror.h
@@ -283,6 +283,8 @@ typedef enum {
VIR_ERR_OPERATION_UNSUPPORTED = 84, /* The requested operation is not
supported */
VIR_ERR_SSH = 85, /* error in ssh transport driver */
+ VIR_ERR_AGENT_UNRESPONSIVE = 86, /* guest agent is unresponsive,
+ not running or not usable */
} virErrorNumber;
/**
diff --git a/src/qemu/qemu_agent.c b/src/qemu/qemu_agent.c
index c658bf8..ba29783 100644
--- a/src/qemu/qemu_agent.c
+++ b/src/qemu/qemu_agent.c
@@ -880,7 +880,7 @@ static int qemuAgentSend(qemuAgentPtr mon,
if ((timeout && virCondWaitUntil(&mon->notify, &mon->lock, then) < 0) ||
(!timeout && virCondWait(&mon->notify, &mon->lock) < 0)) {
if (errno == ETIMEDOUT) {
- virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
+ virReportError(VIR_ERR_AGENT_UNRESPONSIVE, "%s",
_("Guest agent not available for now"));
ret = -2;
} else {
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index 3fcca0e..f64d9ec 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -1734,8 +1734,9 @@ static int qemuDomainShutdownFlags(virDomainPtr dom, unsigned int flags) {
if (useAgent) {
if (priv->agentError) {
- virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
- _("QEMU guest agent is not available due to an error"));
+ virReportError(VIR_ERR_AGENT_UNRESPONSIVE, "%s",
+ _("QEMU guest agent is not "
+ "available due to an error"));
goto cleanup;
}
if (!priv->agent) {
@@ -1815,8 +1816,9 @@ qemuDomainReboot(virDomainPtr dom, unsigned int flags)
if (useAgent) {
if (priv->agentError) {
- virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
- _("QEMU guest agent is not available due to an error"));
+ virReportError(VIR_ERR_AGENT_UNRESPONSIVE, "%s",
+ _("QEMU guest agent is not "
+ "available due to an error"));
goto cleanup;
}
if (!priv->agent) {
@@ -10391,7 +10393,7 @@ qemuDomainSnapshotFSFreeze(struct qemud_driver *driver,
int freezed;
if (priv->agentError) {
- virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
+ virReportError(VIR_ERR_AGENT_UNRESPONSIVE, "%s",
_("QEMU guest agent is not "
"available due to an error"));
return -1;
@@ -10419,7 +10421,7 @@ qemuDomainSnapshotFSThaw(struct qemud_driver *driver,
if (priv->agentError) {
if (report)
- virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
+ virReportError(VIR_ERR_AGENT_UNRESPONSIVE, "%s",
_("QEMU guest agent is not "
"available due to an error"));
return -1;
@@ -13708,8 +13710,9 @@ qemuDomainPMSuspendForDuration(virDomainPtr dom,
}
if (priv->agentError) {
- virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
- _("QEMU guest agent is not available due to an error"));
+ virReportError(VIR_ERR_AGENT_UNRESPONSIVE, "%s",
+ _("QEMU guest agent is not "
+ "available due to an error"));
goto cleanup;
}
@@ -13849,8 +13852,9 @@ qemuDomainAgentCommand(virDomainPtr domain,
}
if (priv->agentError) {
- virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
- _("QEMU guest agent is not available due to an error"));
+ virReportError(VIR_ERR_AGENT_UNRESPONSIVE, "%s",
+ _("QEMU guest agent is not "
+ "available due to an error"));
goto cleanup;
}
diff --git a/src/util/virterror.c b/src/util/virterror.c
index 3ee2ae0..7caa69e 100644
--- a/src/util/virterror.c
+++ b/src/util/virterror.c
@@ -1199,6 +1199,13 @@ virErrorMsg(virErrorNumber error, const char *info)
errmsg = _("SSH transport error");
else
errmsg = _("SSH transport error: %s");
+ break;
+ case VIR_ERR_AGENT_UNRESPONSIVE:
+ if (info == NULL)
+ errmsg = _("Guest agent is not responding");
+ else
+ errmsg = _("Guest agent is not responding: %s");
+ break;
}
return errmsg;
}
--
1.7.8.6
12 years, 2 months
[libvirt] [PATCH libvirt-glib] Don't hold events lock when dispatching free callbacks
by Daniel P. Berrange
From: "Daniel P. Berrange" <berrange(a)redhat.com>
The _event_timeout_remove and _event_handle_remove methods
were holding onto the global lock when invoking the free
callback. This is a violation of the libvirt events API
contract which requires free callbacks to be invoked in
a re-entrant safe context.
---
libvirt-glib/libvirt-glib-event.c | 8 ++------
1 file changed, 2 insertions(+), 6 deletions(-)
diff --git a/libvirt-glib/libvirt-glib-event.c b/libvirt-glib/libvirt-glib-event.c
index 657c1bf..2a9ee23 100644
--- a/libvirt-glib/libvirt-glib-event.c
+++ b/libvirt-glib/libvirt-glib-event.c
@@ -256,13 +256,11 @@ _event_handle_remove(gpointer data)
{
struct gvir_event_handle *h = data;
- g_mutex_lock(eventlock);
-
if (h->ff)
(h->ff)(h->opaque);
+ g_mutex_lock(eventlock);
g_ptr_array_remove_fast(handles, h);
-
g_mutex_unlock(eventlock);
return FALSE;
@@ -414,13 +412,11 @@ _event_timeout_remove(gpointer data)
{
struct gvir_event_timeout *t = data;
- g_mutex_lock(eventlock);
-
if (t->ff)
(t->ff)(t->opaque);
+ g_mutex_lock(eventlock);
g_ptr_array_remove_fast(timeouts, t);
-
g_mutex_unlock(eventlock);
return FALSE;
--
1.7.11.2
12 years, 2 months
[libvirt] [PATCH] conf: prevent NULL pointer access in virSecurityLabelDefsParseXML
by Ján Tomko
When checking for seclabels without security models, def->nseclabels is
already set to n. In the case of an error def->seclabels is freed but
nseclabels is left untouched. This leads to a segmentation fault when
def is freed in virDomainDefParseXML.
---
src/conf/domain_conf.c | 1 +
1 files changed, 1 insertions(+), 0 deletions(-)
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index 82ccf4d..c02d6f8 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -3179,6 +3179,7 @@ error:
virSecurityLabelDefFree(def->seclabels[i - 1]);
}
VIR_FREE(def->seclabels);
+ def->nseclabels = 0;
VIR_FREE(list);
return -1;
}
--
1.7.8.6
12 years, 2 months
[libvirt] [PATCH] qemu: Switch to unified func name
by Michal Privoznik
With the latest patches libvirt supports qemu agent monitor
passthrough. However, function in qemu driver is called
qemuDrvDomainAgentCommand. s/Drv// as used in all other names.
---
src/qemu/qemu_driver.c | 4 ++--
1 files changed, 2 insertions(+), 2 deletions(-)
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index 64c407d..3fcca0e 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -13815,7 +13815,7 @@ qemuListAllDomains(virConnectPtr conn,
}
static char *
-qemuDrvDomainAgentCommand(virDomainPtr domain,
+qemuDomainAgentCommand(virDomainPtr domain,
const char *cmd,
int timeout,
unsigned int flags)
@@ -14028,7 +14028,7 @@ static virDriver qemuDriver = {
.domainSnapshotDelete = qemuDomainSnapshotDelete, /* 0.8.0 */
.qemuDomainMonitorCommand = qemuDomainMonitorCommand, /* 0.8.3 */
.qemuDomainAttach = qemuDomainAttach, /* 0.9.4 */
- .qemuDomainArbitraryAgentCommand = qemuDrvDomainAgentCommand, /* 0.10.0 */
+ .qemuDomainArbitraryAgentCommand = qemuDomainAgentCommand, /* 0.10.0 */
.domainOpenConsole = qemuDomainOpenConsole, /* 0.8.6 */
.domainOpenGraphics = qemuDomainOpenGraphics, /* 0.9.7 */
.domainInjectNMI = qemuDomainInjectNMI, /* 0.9.2 */
--
1.7.8.6
12 years, 3 months
[libvirt] [PATCH] rpc: fix segmentation fault caused by null client-sock
by Guannan Ren
BZ:https://bugzilla.redhat.com/show_bug.cgi?id=851423
The client-sock could have been set to NULL by eventloop thread
after async event fired.
---
src/rpc/virnetclient.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/src/rpc/virnetclient.c b/src/rpc/virnetclient.c
index 4ecc703..43a9814 100644
--- a/src/rpc/virnetclient.c
+++ b/src/rpc/virnetclient.c
@@ -644,7 +644,8 @@ virNetClientMarkClose(virNetClientPtr client,
int reason)
{
VIR_DEBUG("client=%p, reason=%d", client, reason);
- virNetSocketRemoveIOCallback(client->sock);
+ if (client->sock)
+ virNetSocketRemoveIOCallback(client->sock);
client->wantClose = true;
client->closeReason = reason;
}
--
1.7.11.4
12 years, 3 months
[libvirt] [PATCH] qemu: fix regression with pinning
by Martin Kletzander
Commit 4b03d59167f4a4c6ec57def315a61d977466e75b changed the pinning
behavior in a way that makes some machine non-startable.
The comment mentioning that we cannot control each vcpu when there is
no VCPU<->PID mapping available is true, however, this isn't
necessarily an error, because this can be caused by old QEMU without
support for "query-cpus" command as well as a software emulated
machines that don't create more than one process.
---
src/qemu/qemu_cgroup.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/src/qemu/qemu_cgroup.c b/src/qemu/qemu_cgroup.c
index 2237d11..b45bb49 100644
--- a/src/qemu/qemu_cgroup.c
+++ b/src/qemu/qemu_cgroup.c
@@ -567,9 +567,9 @@ int qemuSetupCgroupForVcpu(struct qemud_driver *driver, virDomainObjPtr vm)
/* If we don't know VCPU<->PID mapping or all vcpu runs in the same
* thread, we cannot control each vcpu.
*/
- virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
- _("Unable to get vcpus' pids."));
- goto cleanup;
+ VIR_DEBUG("%s", _("Unable to get vcpus' pids."));
+ virCgroupFree(&cgroup);
+ return 0;
}
for (i = 0; i < priv->nvcpupids; i++) {
--
1.7.12
12 years, 3 months
[libvirt] [PATCH] qemu: fix regression with spice tls port allocation
by Martin Kletzander
In my quest for reusing variables I failed to edit one variable when
fixing details between two patch versions. That results in a failure
to start qemu with autoport and spice tls, because qemu is trying to
bind two sockets to the same port.
---
src/qemu/qemu_process.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c
index abe6e74..7f85aea 100644
--- a/src/qemu/qemu_process.c
+++ b/src/qemu/qemu_process.c
@@ -3500,7 +3500,7 @@ int qemuProcessStart(virConnectPtr conn,
goto cleanup;
}
- vm->def->graphics[0]->data.spice.tlsPort = port;
+ vm->def->graphics[0]->data.spice.tlsPort = tlsPort;
}
}
--
1.7.12
12 years, 3 months
[libvirt] [PATCH] virsh: fix missing return value
by Alex Jia
Although virsh command raises a correct error information, the command status
returns 0(true), this patch is used for fixing this issue.
Signed-off-by: Alex Jia <ajia(a)redhat.com>
---
tools/virsh-host.c | 3 +++
1 files changed, 3 insertions(+), 0 deletions(-)
diff --git a/tools/virsh-host.c b/tools/virsh-host.c
index da60895..2f3b413 100644
--- a/tools/virsh-host.c
+++ b/tools/virsh-host.c
@@ -701,11 +701,13 @@ cmdQemuAgentCommand(vshControl *ctl, const vshCmd *cmd)
judge = vshCommandOptInt(cmd, "timeout", &timeout);
if (judge < 0) {
vshError(ctl, "%s", _("timeout number has to be a number"));
+ goto cleanup;
} else if (judge > 0) {
judge = 1;
}
if (judge && timeout < 1) {
vshError(ctl, "%s", _("timeout must be positive"));
+ goto cleanup;
}
if (vshCommandOptBool(cmd, "async")) {
@@ -719,6 +721,7 @@ cmdQemuAgentCommand(vshControl *ctl, const vshCmd *cmd)
if (judge > 1) {
vshError(ctl, "%s", _("timeout, async and block options are exclusive"));
+ goto cleanup;
}
result = virDomainQemuAgentCommand(dom, guest_agent_cmd, timeout, flags);
--
1.7.1
12 years, 3 months
[libvirt] [PATCH] specfile: require libnl3 for Fedora >= 18 and RHEL >= 7
by Laine Stump
Everything is ready in both netcf and libvirt to switch over to libnl3
in future releases of both Fedora and RHEL. This needs to be done more
or less simultaneously in both packages, though, because you can't mix
libnl1.1 and libnl3 in the same process (e.g. libvirtd using
libnl-3.so and libnetcf.so, while libnetcf.so uses libnl.so)
This patch does two things when fedora >= 18 || rhel >= 7):
1) requires libnl3-devel
2) requires netcf-devel-0.2.2 or greater
(the idea is that a similar patch is going into netcf's specfile, so
that when a build of netcf is done on F18 or later (or RHEL7 or later)
netcf will be guaranteed to be built with libnl3 rather than
libnl-1.1)
---
libvirt.spec.in | 9 +++++++++
1 file changed, 9 insertions(+)
diff --git a/libvirt.spec.in b/libvirt.spec.in
index 18a7fb8..4b4d1d0 100644
--- a/libvirt.spec.in
+++ b/libvirt.spec.in
@@ -409,8 +409,12 @@ BuildRequires: sanlock-devel >= 1.8
BuildRequires: libpcap-devel
%endif
%if %{with_libnl}
+%if 0%{?fedora} >= 18 || 0%{?rhel} >= 7
+BuildRequires: libnl3-devel
+%else
BuildRequires: libnl-devel
%endif
+%endif
%if %{with_avahi}
BuildRequires: avahi-devel
%endif
@@ -489,13 +493,18 @@ BuildRequires: libcap-ng-devel >= 0.5.0
%if %{with_phyp}
BuildRequires: libssh2-devel
%endif
+
%if %{with_netcf}
+%if 0%{?fedora} >= 18 || 0%{?rhel} >= 7
+BuildRequires: netcf-devel >= 0.2.2
+%else
%if 0%{?fedora} >= 16 || 0%{?rhel} >= 6
BuildRequires: netcf-devel >= 0.1.8
%else
BuildRequires: netcf-devel >= 0.1.4
%endif
%endif
+%endif
%if %{with_esx}
%if 0%{?fedora} >= 9 || 0%{?rhel} >= 6
BuildRequires: libcurl-devel
--
1.7.11.4
12 years, 3 months
