[libvirt] [PATCH] util: fix crash in virClassIsDerivedFrom for CloseCallbacks objects
by Maxim Nestratov
There is a possibility that qemu driver frees by unreferencing its
closeCallbacks pointer as it has the only reference to the object,
while in fact not all users of CloseCallbacks called thier
virCloseCallbacksUnset.
Backtrace is the following:
Thread #1:
0 in pthread_cond_wait@(a)GLIBC_2.3.2 () from /lib64/libpthread.so.0
1 in virCondWait (c=<optimized out>, m=<optimized out>)
at util/virthread.c:154
2 in virThreadPoolFree (pool=0x7f0810110b50)
at util/virthreadpool.c:266
3 in qemuStateCleanup () at qemu/qemu_driver.c:1116
4 in virStateCleanup () at libvirt.c:808
5 in main (argc=<optimized out>, argv=<optimized out>)
at libvirtd.c:1660
Thread #2:
0 in virClassIsDerivedFrom (klass=0xdeadbeef, parent=0x7f0837c694d0) at util/virobject.c:169
1 in virObjectIsClass (anyobj=anyobj@entry=0x7f08101d4760, klass=<optimized out>) at util/virobject.c:365
2 in virObjectLock (anyobj=0x7f08101d4760) at util/virobject.c:317
3 in virCloseCallbacksUnset (closeCallbacks=0x7f08101d4760, vm=vm@entry=0x7f08101d47b0, cb=cb@entry=0x7f081d078fc0 <qemuProcessAutoDestroy>) at util/virclosecallbacks.c:163
4 in qemuProcessAutoDestroyRemove (driver=driver@entry=0x7f081018be50, vm=vm@entry=0x7f08101d47b0) at qemu/qemu_process.c:6368
5 in qemuProcessStop (driver=driver@entry=0x7f081018be50, vm=vm@entry=0x7f08101d47b0, reason=reason@entry=VIR_DOMAIN_SHUTOFF_SHUTDOWN, asyncJob=asyncJob@entry=QEMU_ASYNC_JOB_NONE, flags=flags@entry=0) at qemu/qemu_process.c:5854
6 in processMonitorEOFEvent (vm=0x7f08101d47b0, driver=0x7f081018be50) at qemu/qemu_driver.c:4585
7 qemuProcessEventHandler (data=<optimized out>, opaque=0x7f081018be50) at qemu/qemu_driver.c:4629
8 in virThreadPoolWorker (opaque=opaque@entry=0x7f0837c4f820) at util/virthreadpool.c:145
9 in virThreadHelper (data=<optimized out>) at util/virthread.c:206
10 in start_thread () from /lib64/libpthread.so.0
Let's reference CloseCallbacks object in virCloseCallbacksSet and
unreference in virCloseCallbacksUnset.
Signed-off-by: Maxim Nestratov <mnestratov(a)virtuozzo.com>
---
src/util/virclosecallbacks.c | 3 +++
1 file changed, 3 insertions(+)
diff --git a/src/util/virclosecallbacks.c b/src/util/virclosecallbacks.c
index 82d4071..891a92b 100644
--- a/src/util/virclosecallbacks.c
+++ b/src/util/virclosecallbacks.c
@@ -141,6 +141,7 @@ virCloseCallbacksSet(virCloseCallbacksPtr closeCallbacks,
virObjectRef(vm);
}
+ virObjectRef(closeCallbacks);
ret = 0;
cleanup:
virObjectUnlock(closeCallbacks);
@@ -180,6 +181,8 @@ virCloseCallbacksUnset(virCloseCallbacksPtr closeCallbacks,
ret = 0;
cleanup:
virObjectUnlock(closeCallbacks);
+ if (!ret)
+ virObjectUnref(closeCallbacks);
return ret;
}
--
2.4.11
8 years, 2 months
[libvirt] [PATCH] qemu: guest agent: introduce new error code VIR_ERR_AGENT_UNSYNCED
by Maxim Nestratov
From: Yuri Pudgorodskiy <yur(a)virtuozzo.com>
A separate error code will help recognize real failures from
necessity to try again
Signed-off-by: Maxim Nestratov <mnestratov(a)virtuozzo.com>
---
include/libvirt/virterror.h | 2 ++
src/qemu/qemu_agent.c | 6 +++---
src/util/virerror.c | 6 ++++++
3 files changed, 11 insertions(+), 3 deletions(-)
diff --git a/include/libvirt/virterror.h b/include/libvirt/virterror.h
index 2ec560e..efe83aa 100644
--- a/include/libvirt/virterror.h
+++ b/include/libvirt/virterror.h
@@ -315,6 +315,8 @@ typedef enum {
VIR_ERR_AUTH_UNAVAILABLE = 94, /* authentication unavailable */
VIR_ERR_NO_SERVER = 95, /* Server was not found */
VIR_ERR_NO_CLIENT = 96, /* Client was not found */
+ VIR_ERR_AGENT_UNSYNCED = 97, /* guest agent replies with wrong id
+ to guest-sync command */
} virErrorNumber;
/**
diff --git a/src/qemu/qemu_agent.c b/src/qemu/qemu_agent.c
index eeede6b..fdc4fed 100644
--- a/src/qemu/qemu_agent.c
+++ b/src/qemu/qemu_agent.c
@@ -966,21 +966,21 @@ qemuAgentGuestSync(qemuAgentPtr mon)
goto cleanup;
if (!sync_msg.rxObject) {
- virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
+ virReportError(VIR_ERR_AGENT_UNSYNCED, "%s",
_("Missing monitor reply object"));
goto cleanup;
}
if (virJSONValueObjectGetNumberUlong(sync_msg.rxObject,
"return", &id_ret) < 0) {
- virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
+ virReportError(VIR_ERR_AGENT_UNSYNCED, "%s",
_("Malformed return value"));
goto cleanup;
}
VIR_DEBUG("Guest returned ID: %llu", id_ret);
if (id_ret != id) {
- virReportError(VIR_ERR_INTERNAL_ERROR,
+ virReportError(VIR_ERR_AGENT_UNSYNCED,
_("Guest agent returned ID: %llu instead of %llu"),
id_ret, id);
goto cleanup;
diff --git a/src/util/virerror.c b/src/util/virerror.c
index 1177570..2958308 100644
--- a/src/util/virerror.c
+++ b/src/util/virerror.c
@@ -1394,6 +1394,12 @@ virErrorMsg(virErrorNumber error, const char *info)
else
errmsg = _("Client not found: %s");
break;
+ case VIR_ERR_AGENT_UNSYNCED:
+ if (info == NULL)
+ errmsg = _("guest agent replied with wrong id to guest-sync command");
+ else
+ errmsg = _("guest agent replied with wrong id to guest-sync command: %s");
+ break;
}
return errmsg;
}
--
2.4.11
8 years, 2 months
[libvirt] [PATCH] virsh: use virConnectGetDomainCapabilities with maxvcpus
by Shivaprasad G Bhat
virsh maxvcpus --type kvm output is useless on PPC. Also, in
commit e6806d79 we documented not rely on virConnectGetMaxVcpus
output. Fix the maxvcpus to use virConnectGetDomainCapabilities
now to make it useful. The call is made to use the default emulator
binary and to check for the host machine and arch which is what the
command intends to show anyway.
Signed-off-by: Shivaprasad G Bhat <sbhat(a)linux.vnet.ibm.com>
---
tools/virsh-host.c | 28 +++++++++++++++++++++++++---
1 file changed, 25 insertions(+), 3 deletions(-)
diff --git a/tools/virsh-host.c b/tools/virsh-host.c
index 57f0c0e..505cfbb 100644
--- a/tools/virsh-host.c
+++ b/tools/virsh-host.c
@@ -607,16 +607,38 @@ cmdMaxvcpus(vshControl *ctl, const vshCmd *cmd)
{
const char *type = NULL;
int vcpus;
+ char *caps = NULL;
+ const unsigned int flags = 0; /* No flags so far */
+ xmlDocPtr xml = NULL;
+ xmlXPathContextPtr ctxt = NULL;
virshControlPtr priv = ctl->privData;
if (vshCommandOptStringReq(ctl, cmd, "type", &type) < 0)
return false;
- if ((vcpus = virConnectGetMaxVcpus(priv->conn, type)) < 0)
- return false;
+ caps = virConnectGetDomainCapabilities(priv->conn, NULL, NULL, NULL, type, flags);
+ if (caps) {
+ xml = virXMLParseStringCtxt(caps, _("(domainCapabilities)"), &ctxt);
+ if (!xml) {
+ VIR_FREE(caps);
+ return false;
+ }
- vshPrint(ctl, "%d\n", vcpus);
+ virXPathInt("string(./vcpu[1]/@max)", ctxt, &vcpus);
+ xmlXPathFreeContext(ctxt);
+ xmlFreeDoc(xml);
+ VIR_FREE(caps);
+ } else {
+ if (last_error && last_error->code != VIR_ERR_NO_SUPPORT)
+ return false;
+
+ vshResetLibvirtError();
+ if ((vcpus = virConnectGetMaxVcpus(priv->conn, type)) < 0)
+ return false;
+ }
+
+ vshPrint(ctl, "%d\n", vcpus);
return true;
}
8 years, 2 months
[libvirt] [PATCH] tools: Pass opaque data in vshCompleter and introduce autoCompleteOpaque
by Nishith Shah
This patch changes the signature of vshCompleters, allowing to pass along
some data that we might want to along with the completers; for example,
we might want to pass the autocomplete vshControl along with the
completer, in case the completer requires a connection to libvirtd.
Signed-off-by: Nishith Shah <nishithshah.2211(a)gmail.com>
---
tools/vsh.c | 10 +++++++++-
tools/vsh.h | 2 +-
2 files changed, 10 insertions(+), 2 deletions(-)
diff --git a/tools/vsh.c b/tools/vsh.c
index 3157922..3cc03ec 100644
--- a/tools/vsh.c
+++ b/tools/vsh.c
@@ -64,6 +64,11 @@
# define SA_SIGINFO 0
#endif
+#ifdef WITH_READLINE
+/* For autocompletion */
+void *autoCompleteOpaque;
+#endif
+
/* NOTE: It would be much nicer to have these two as part of vshControl
* structure, unfortunately readline doesn't support passing opaque data
* and only relies on static data accessible from the user-side callback
@@ -2808,7 +2813,8 @@ vshReadlineParse(const char *text, int state)
res = vshReadlineOptionsGenerator(sanitized_text, state, cmd);
} else if (non_bool_opt_exists && data_complete && opt->completer) {
if (!completed_list)
- completed_list = opt->completer(opt->completer_flags);
+ completed_list = opt->completer(autoCompleteOpaque,
+ opt->completer_flags);
if (completed_list) {
while ((completed_name = completed_list[completed_list_index])) {
completed_list_index++;
@@ -2858,6 +2864,8 @@ vshReadlineInit(vshControl *ctl)
char *histsize_env = NULL;
const char *histsize_str = NULL;
+ /* Opaque data for autocomplete callbacks and connections to libvirtd */
+ autoCompleteOpaque = ctl;
/* Allow conditional parsing of the ~/.inputrc file.
* Work around ancient readline 4.1 (hello Mac OS X),
* which declared it as 'char *' instead of 'const char *'.
diff --git a/tools/vsh.h b/tools/vsh.h
index 7f43055..e53910f 100644
--- a/tools/vsh.h
+++ b/tools/vsh.h
@@ -123,7 +123,7 @@ typedef struct _vshCmdOpt vshCmdOpt;
typedef struct _vshCmdOptDef vshCmdOptDef;
typedef struct _vshControl vshControl;
-typedef char **(*vshCompleter)(unsigned int flags);
+typedef char **(*vshCompleter)(void *opaque, unsigned int flags);
/*
* vshCmdInfo -- name/value pair for information about command
--
2.1.4
8 years, 2 months
[libvirt] [PATCH] util: hostcpu: improve CPU freq code for FreeBSD
by Roman Bogorodskiy
Current implementation uses the dev.cpu.0.freq sysctl that is
provided by the cpufreq(4) framework and returns the actual
CPU frequency. However, there are environment where it's not available,
e.g. when running nested in KVM. In this case fall back to hw.clockrate
that reports CPU frequency at the boot time.
Resolves (hopefully):
https://bugzilla.redhat.com/show_bug.cgi?id=1369964
---
src/util/virhostcpu.c | 11 +++++++++--
1 file changed, 9 insertions(+), 2 deletions(-)
diff --git a/src/util/virhostcpu.c b/src/util/virhostcpu.c
index 0f03ff8..b5a37a8 100644
--- a/src/util/virhostcpu.c
+++ b/src/util/virhostcpu.c
@@ -994,9 +994,16 @@ virHostCPUGetInfo(virArch hostarch ATTRIBUTE_UNUSED,
*threads = 1;
# ifdef __FreeBSD__
+ /* dev.cpu.%d.freq reports current active CPU frequency. It is provided by
+ * the cpufreq(4) framework. However, it might be disabled or no driver
+ * available. In this case fallback to "hw.clockrate" which reports boot time
+ * CPU frequency. */
+
if (sysctlbyname("dev.cpu.0.freq", &cpu_freq, &cpu_freq_len, NULL, 0) < 0) {
- virReportSystemError(errno, "%s", _("cannot obtain CPU freq"));
- return -1;
+ if (sysctlbyname("hw.clockrate", &cpu_freq, &cpu_freq_len, NULL, 0) < 0) {
+ virReportSystemError(errno, "%s", _("cannot obtain CPU freq"));
+ return -1;
+ }
}
*mhz = cpu_freq;
--
2.7.4
8 years, 2 months
[libvirt] [PATCH] virsh: Fix *-event error string
by Christophe Fergeau
When using
virsh net-event non-existing-net
the error message says that 'either --list or event type is required'
This is misleading as 'virsh net-event $valid-event-type' is not going
to work either. What is expected is 'virsh net-event --event
$valid-event-type'
This commit fixes the string in pool-event, nodedev-event, event, and
net-event.
---
tools/virsh-domain.c | 2 +-
tools/virsh-network.c | 2 +-
tools/virsh-nodedev.c | 2 +-
tools/virsh-pool.c | 2 +-
4 files changed, 4 insertions(+), 4 deletions(-)
diff --git a/tools/virsh-domain.c b/tools/virsh-domain.c
index a614512..702a8bd8 100644
--- a/tools/virsh-domain.c
+++ b/tools/virsh-domain.c
@@ -12694,7 +12694,7 @@ cmdEvent(vshControl *ctl, const vshCmd *cmd)
}
} else if (!all) {
vshError(ctl, "%s",
- _("one of --list, --all, or event type is required"));
+ _("one of --list, --all, or --event type is required"));
return false;
}
diff --git a/tools/virsh-network.c b/tools/virsh-network.c
index eec7faf..c6bd132 100644
--- a/tools/virsh-network.c
+++ b/tools/virsh-network.c
@@ -1238,7 +1238,7 @@ cmdNetworkEvent(vshControl *ctl, const vshCmd *cmd)
if (vshCommandOptStringReq(ctl, cmd, "event", &eventName) < 0)
return false;
if (!eventName) {
- vshError(ctl, "%s", _("either --list or event type is required"));
+ vshError(ctl, "%s", _("either --list or --event type is required"));
return false;
}
if ((event = virshNetworkEventIdTypeFromString(eventName)) < 0) {
diff --git a/tools/virsh-nodedev.c b/tools/virsh-nodedev.c
index 805c0ff..0e695b9 100644
--- a/tools/virsh-nodedev.c
+++ b/tools/virsh-nodedev.c
@@ -903,7 +903,7 @@ cmdNodeDeviceEvent(vshControl *ctl, const vshCmd *cmd)
if (vshCommandOptStringReq(ctl, cmd, "event", &eventName) < 0)
return false;
if (!eventName) {
- vshError(ctl, "%s", _("either --list or event type is required"));
+ vshError(ctl, "%s", _("either --list or --event type is required"));
return false;
}
diff --git a/tools/virsh-pool.c b/tools/virsh-pool.c
index d25851e..70b2bdd 100644
--- a/tools/virsh-pool.c
+++ b/tools/virsh-pool.c
@@ -2057,7 +2057,7 @@ cmdPoolEvent(vshControl *ctl, const vshCmd *cmd)
if (vshCommandOptStringReq(ctl, cmd, "event", &eventName) < 0)
return false;
if (!eventName) {
- vshError(ctl, "%s", _("either --list or event type is required"));
+ vshError(ctl, "%s", _("either --list or --event type is required"));
return false;
}
--
2.7.4
8 years, 2 months
[libvirt] qapi DEVICE_DELETED event issued *before* instance_finalize?!
by Alex Williamson
Hey,
I'm out of my QOM depth, so I'll just beg for help in advance. I
noticed in testing vfio-pci hotunplug that the host seems to be trying
to reclaim the device before QEMU is actually done with it, there's a
very short race where libvirt has seen the DEVICE_DELETED event and
tries to unbind the physical device from vfio-pci, the use count is
clearly non-zero because the host driver tries to send a device
request, but that event channel has already been torn down. Nearly
immediately after, QEMU finally releases the device, but we can't do a
proper reset due to some issues with device references in the kernel.
When I run gdb on QEMU with breakpoints at
qapi_event_send_device_deleted() and vfio_instance_finalize(), the
QAPI even happens first. Clearly this is horribly wrong, right? I
can't unmap my references to the vfio device file until my
instance_finalize is called, so I'm always going to have that open when
libvirt takes the DEVICE_DELETED event as a cue to return the device to
host drivers. The call chains look like this:
#0 qapi_event_send_device_deleted (has_device=true,
device=0x7f5ca3e36fb0 "hostdev0",
path=0x7f5c89e84fe0 "/machine/peripheral/hostdev0",
errp=0x7f5ca241f9e8 <error_abort>) at qapi-event.c:412
#1 0x00007f5ca1701608 in device_unparent (obj=0x7f5ca43ffc00)
at hw/core/qdev.c:1115
#2 0x00007f5ca18b7891 in object_finalize_child_property (obj=0x7f5ca380f500,
name=0x7f5ca3f21da0 "hostdev0", opaque=0x7f5ca43ffc00) at qom/object.c:1362
#3 0x00007f5ca18b56b2 in object_property_del_child (obj=0x7f5ca380f500,
child=0x7f5ca43ffc00, errp=0x0) at qom/object.c:422
#4 0x00007f5ca18b5790 in object_unparent (obj=0x7f5ca43ffc00)
at qom/object.c:441
#5 0x00007f5ca16c1f31 in acpi_pcihp_eject_slot (s=0x7f5ca4c41268, bsel=0,
slots=4) at hw/acpi/pcihp.c:139
#0 vfio_instance_finalize (obj=0x7f5ca43ffc00)
at /net/gimli/home/alwillia/Work/qemu.git/hw/vfio/pci.c:2731
#1 0x00007f5ca18b57c0 in object_deinit (obj=0x7f5ca43ffc00,
type=0x7f5ca376f490) at qom/object.c:448
#2 0x00007f5ca18b5831 in object_finalize (data=0x7f5ca43ffc00)
at qom/object.c:462
#3 0x00007f5ca18b6782 in object_unref (obj=0x7f5ca43ffc00) at qom/object.c:896
#4 0x00007f5ca1550cc0 in memory_region_unref (mr=0x7f5ca43fff00)
at /net/gimli/home/alwillia/Work/qemu.git/memory.c:1476
#5 0x00007f5ca1553886 in do_address_space_destroy (as=0x7f5ca43ffe10)
at /net/gimli/home/alwillia/Work/qemu.git/memory.c:2272
It appears that DEVICE_DELETED only means the VM is done with the
device but libvirt is interpreting it as QEMU is done with the device.
Which is correct? Do we need a new event or do we need to fix the
ordering of this event? An ordering fix would be more compatible with
existing libvirt. Thanks,
Alex
8 years, 2 months
[libvirt] virt-admin commands aliases
by Erik Skultety
Hi there,
after my presentation at KVM Forum, it was pointed out from the audience
that we might think about doing something about the naming of the
virt-admin's comands, since there is some sort of inconsistency: srv-
vs. client- vs. dmn- (not merged yet). When I sent patches to upstream I
already knew that the naming was not optimal, but I didn't come up with
anything better so I hoped that the reviewer might think of something
better which unfortunately did not happen.
Anyway, there are multiple options how this can be approached but I'm
not 100% satisfied with neither of them:
1) rename the commands completely
Although clean, obviously this is the non-preferred option because this
would break any backwards compatibility however, I think there is a fair
chance that people haven't actually started using it yet (although that
might change between 7.3 and 7.4).
2) create aliases for non-abbreviated forms of the commands
That way, srv- would become server- and dmn- would become daemon-.
However, by doing this we'll end up with 6 almost identical entries in
the commands structure which might be error-prone once we decide to
add/create&add a flag to the command primitive, since the flag would
have to be added both to the alias and to the original (unlikely, but
possible that someone might forget about that)
3) abbreviate client- to something like clnt-
Identical to the above except for the amount of duplicate entries which
would be reduced to 2
4) leave it as is if such a consensus is reached and accepted
I guess this does no need any additional comments.
Thanks for any ideas.
Erik
8 years, 2 months
[libvirt] [PATCH] qemu: Don't warn about missing device in DEVICE_DELETED event
by Jiri Denemark
Debug priority is good enough for this.
Signed-off-by: Jiri Denemark <jdenemar(a)redhat.com>
---
src/qemu/qemu_monitor_json.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/src/qemu/qemu_monitor_json.c b/src/qemu/qemu_monitor_json.c
index 3d0a120..8384314 100644
--- a/src/qemu/qemu_monitor_json.c
+++ b/src/qemu/qemu_monitor_json.c
@@ -941,7 +941,7 @@ qemuMonitorJSONHandleDeviceDeleted(qemuMonitorPtr mon, virJSONValuePtr data)
const char *device;
if (!(device = virJSONValueObjectGetString(data, "device"))) {
- VIR_WARN("missing device in device deleted event");
+ VIR_DEBUG("missing device in device deleted event");
return;
}
--
2.10.0
8 years, 2 months
[libvirt] [PATCH 0/2] util: storage: Fixes for the JSON pseudo protocol parser
by Peter Krempa
Gluster protocol type was not set properly and the RBD protocol was missing.
Peter Krempa (2):
util: storage: Properly set protocol type when parsing gluster json
string
util: storage: Add json pseudo protocol support for legacy RBD strings
src/util/virstoragefile.c | 26 ++++++++++++++++++++++++++
tests/virstoragetest.c | 10 ++++++++--
2 files changed, 34 insertions(+), 2 deletions(-)
--
2.9.2
8 years, 2 months
