[libvirt] [PATCH] Make error reporting for QEMU JSON mode more friendly
by Daniel P. Berrange
Current error reporting for JSON mode returns the full JSON
command string and full JSON error string. This is not very
user friendly, so this change makes the error report only
contain the basic command name, and friendly error message
description string. The full JSON data is logged instead.
* src/qemu/qemu_monitor_json.c: Always return the 'desc' field from
the JSON error message to users.
---
src/qemu/qemu_monitor_json.c | 58 ++++++++++++++++++++++++++++-------------
1 files changed, 39 insertions(+), 19 deletions(-)
diff --git a/src/qemu/qemu_monitor_json.c b/src/qemu/qemu_monitor_json.c
index f0dcf81..9875f38 100644
--- a/src/qemu/qemu_monitor_json.c
+++ b/src/qemu/qemu_monitor_json.c
@@ -246,15 +246,33 @@ qemuMonitorJSONCommand(qemuMonitorPtr mon,
*
* XXX see qerror.h for different klasses & fill out useful params
*/
-static char *qemuMonitorJSONStringifyError(virJSONValuePtr error)
+static const char *
+qemuMonitorJSONStringifyError(virJSONValuePtr error)
{
const char *klass = virJSONValueObjectGetString(error, "class");
+ const char *detail = NULL;
- if (klass) {
- return strdup(klass);
- } else {
- return strdup(_("Missing QEMU error klass"));
- }
+ /* The QMP 'desc' field is usually sufficient for our generic
+ * error reporting needs.
+ */
+ if (klass)
+ detail = virJSONValueObjectGetString(error, "desc");
+
+
+ if (!detail)
+ detail = "unknown QEMU command error";
+
+ return detail;
+}
+
+static const char *
+qemuMonitorJSONCommandName(virJSONValuePtr cmd)
+{
+ const char *name = virJSONValueObjectGetString(cmd, "execute");
+ if (name)
+ return name;
+ else
+ return "<unknown>";
}
static int
@@ -266,20 +284,21 @@ qemuMonitorJSONCheckError(virJSONValuePtr cmd,
char *cmdstr = virJSONValueToString(cmd);
char *replystr = virJSONValueToString(reply);
- if (!error) {
- VIR_DEBUG("Saw a JSON error, but value is null for %s: %s",
- cmdstr, replystr);
+ /* Log the full JSON formatted command & error */
+ VIR_DEBUG("unable to execute QEMU command %s: %s",
+ cmdstr, replystr);
+
+ /* Only send the user the command name + friendly error */
+ if (!error)
qemuReportError(VIR_ERR_INTERNAL_ERROR,
- _("error running QEMU command '%s': '%s'"),
- cmdstr, replystr);
- } else {
- VIR_DEBUG("Got a JSON error set for %s", cmdstr);
- char *detail = qemuMonitorJSONStringifyError(error);
+ _("unable to execute QEMU command '%s'"),
+ qemuMonitorJSONCommandName(cmd));
+ else
qemuReportError(VIR_ERR_INTERNAL_ERROR,
- _("error running QEMU command '%s': %s ('%s')"),
- cmdstr, detail, replystr);
- VIR_FREE(detail);
- }
+ _("unable to execute QEMU command '%s': %s"),
+ qemuMonitorJSONCommandName(cmd),
+ qemuMonitorJSONStringifyError(error));
+
VIR_FREE(cmdstr);
VIR_FREE(replystr);
return -1;
@@ -290,7 +309,8 @@ qemuMonitorJSONCheckError(virJSONValuePtr cmd,
VIR_DEBUG("Neither 'return' nor 'error' is set in the JSON reply %s: %s",
cmdstr, replystr);
qemuReportError(VIR_ERR_INTERNAL_ERROR,
- _("error running QEMU command '%s': '%s'"), cmdstr, replystr);
+ _("unable to execute QEMU command '%s'"),
+ qemuMonitorJSONCommandName(cmd));
VIR_FREE(cmdstr);
VIR_FREE(replystr);
return -1;
--
1.6.6
14 years, 9 months
[libvirt] [PATCH] Fix check for primary IDE controller in QEMU PCI slot assignment
by Daniel P. Berrange
A typo in the check for the primary IDE controller could cause
a crash on restore depending on the exact guest config.
* src/qemu/qemu_conf.c: Fix s/video/controller/ typo & slot
number typo
---
src/qemu/qemu_conf.c | 8 ++++----
1 files changed, 4 insertions(+), 4 deletions(-)
diff --git a/src/qemu/qemu_conf.c b/src/qemu/qemu_conf.c
index 00f761d..dd5efd6 100644
--- a/src/qemu/qemu_conf.c
+++ b/src/qemu/qemu_conf.c
@@ -2085,10 +2085,10 @@ qemuAssignDevicePCISlots(virDomainDefPtr def, qemuDomainPCIAddressSetPtr addrs)
if (def->controllers[i]->type == VIR_DOMAIN_CONTROLLER_TYPE_IDE &&
def->controllers[i]->idx == 0) {
if (def->controllers[i]->info.type == VIR_DOMAIN_DEVICE_ADDRESS_TYPE_PCI) {
- if (def->videos[i]->info.addr.pci.domain != 0 ||
- def->videos[i]->info.addr.pci.bus != 0 ||
- def->videos[i]->info.addr.pci.slot != 2 ||
- def->videos[i]->info.addr.pci.function != 0) {
+ if (def->controllers[i]->info.addr.pci.domain != 0 ||
+ def->controllers[i]->info.addr.pci.bus != 0 ||
+ def->controllers[i]->info.addr.pci.slot != 1 ||
+ def->controllers[i]->info.addr.pci.function != 1) {
qemuReportError(VIR_ERR_INTERNAL_ERROR, "%s",
_("Primary IDE controller must have PCI address 0:0:1.1"));
goto error;
--
1.6.6
14 years, 9 months
[libvirt] [PATCH] interface: Use proper return codes in the open function
by Matthias Bolte
The open function returned -1 in case of an error, but -1 maps
to VIR_DRV_OPEN_DECLINED instead of VIR_DRV_OPEN_ERROR.
---
src/interface/netcf_driver.c | 4 ++--
1 files changed, 2 insertions(+), 2 deletions(-)
diff --git a/src/interface/netcf_driver.c b/src/interface/netcf_driver.c
index 2753049..7f4d43d 100644
--- a/src/interface/netcf_driver.c
+++ b/src/interface/netcf_driver.c
@@ -139,7 +139,7 @@ static virDrvOpenStatus interfaceOpenInterface(virConnectPtr conn,
}
conn->interfacePrivateData = driverState;
- return 0;
+ return VIR_DRV_OPEN_SUCCESS;
netcf_error:
if (driverState->netcf)
@@ -150,7 +150,7 @@ netcf_error:
mutex_error:
VIR_FREE(driverState);
alloc_error:
- return -1;
+ return VIR_DRV_OPEN_ERROR;
}
static int interfaceCloseInterface(virConnectPtr conn)
--
1.6.3.3
14 years, 9 months
Re: [libvirt] Cannot enumerate physical devices with libvirt 0.7.6
by Frédéric Grelot
> > # libvirtd --version
> > libvirtd (libvirt) 0.7.6
> >
> > # virsh --version
> > 0.7.6
> >
> > # virsh nodedev-list
> > error :Failed to count node devices
> > error :this function is not supported by the hypervisor:
> virNodeNumOfDevices
I restarted the server, with no change.
I uninstalled and reinstalled libvirt, same result.
I had a look at libvirt logs, there is nothing special.
I tried several other things, with no luck.
I check that "lshal" gives result : it is actually the case...
> It would appear that the udev driver failed to start - there are
> probably
> messages in syslog about the problem
I'll check that.
However I don't know what happened with this last update, physical devices worked great before it...
If what you say is right, it may be linked with some other update that occured at the same time...
Thanks for you help!
>
> Regards,
> Daniel
> --
> |: Red Hat, Engineering, London -o-
> http://people.redhat.com/berrange/ :|
> |: http://libvirt.org -o- http://virt-manager.org -o-
> http://ovirt.org :|
> |: http://autobuild.org -o-
> http://search.cpan.org/~danberr/ :|
> |: GnuPG: 7D3B9505 -o- F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B
> 9505 :|
14 years, 9 months
[libvirt] Cannot enumerate physical devices with libvirt 0.7.6
by Frédéric Grelot
Hi all,
I often update my F12 box with the rawvirt repository, which just installed libvirt 0.7.6. Since that, I cannot enumerate physical devices (and thus get a beautiful error when I start one of my vms that should use a USB device).
I don't think I changed anything else on that box, and, of course, I already restarted the libvirt daemon.
I haven't restarted the server itself, because it is currently used, but I could do it if necessary...
I make good use of the qemu hypervisor, and I'm looking forward to solve this issue!
Frederic.
# libvirtd --version
libvirtd (libvirt) 0.7.6
# virsh --version
0.7.6
# virsh nodedev-list
error :Failed to count node devices
error :this function is not supported by the hypervisor: virNodeNumOfDevices
14 years, 9 months
[libvirt] [PATCH] Convert virSecurityReportError into a macro
by Matthias Bolte
---
src/security/security_driver.c | 18 ------------------
src/security/security_driver.h | 6 +++---
2 files changed, 3 insertions(+), 21 deletions(-)
diff --git a/src/security/security_driver.c b/src/security/security_driver.c
index 27945a6..4c98190 100644
--- a/src/security/security_driver.c
+++ b/src/security/security_driver.c
@@ -90,24 +90,6 @@ virSecurityDriverStartup(virSecurityDriverPtr *drv,
return -2;
}
-void
-virSecurityReportError(int code, const char *fmt, ...)
-{
- va_list args;
- char errorMessage[1024];
-
- if (fmt) {
- va_start(args, fmt);
- vsnprintf(errorMessage, sizeof(errorMessage) - 1, fmt, args);
- va_end(args);
- } else
- errorMessage[0] = '\0';
-
- virRaiseError(NULL, NULL, NULL, VIR_FROM_SECURITY, code,
- VIR_ERR_ERROR, NULL, NULL, NULL, -1, -1, "%s",
- errorMessage);
-}
-
/*
* Helpers
*/
diff --git a/src/security/security_driver.h b/src/security/security_driver.h
index 8860d81..15671b3 100644
--- a/src/security/security_driver.h
+++ b/src/security/security_driver.h
@@ -88,9 +88,9 @@ int virSecurityDriverStartup(virSecurityDriverPtr *drv,
int
virSecurityDriverVerify(virDomainDefPtr def);
-void
-virSecurityReportError(int code, const char *fmt, ...)
- ATTRIBUTE_FMT_PRINTF(2, 3);
+#define virSecurityReportError(code, fmt...) \
+ virReportErrorHelper(NULL, VIR_FROM_SECURITY, code, __FILE__, \
+ __FUNCTION__, __LINE__, fmt)
/* Helpers */
void virSecurityDriverInit(virSecurityDriverPtr drv);
--
1.6.3.3
14 years, 9 months
[libvirt] How to shut down every VM when libvirt is stopped
by Laurent Léonard
Hi,
With libvirt 0.4.6 for example, the behaviour when we stopped libvirt was to
shut down every VM. That was a problem in many cases (upgrades, etc.) but
that behaviour was very useful with high avalability systems like Heartbeat.
With an Hearbeat + DRBD system, if the libvirt service is stopped by
Heartbeat, every VM should be stopped too. Otherwise we will experience
problems and disk corruption.
Is there any way to configure libvirt to have that old behaviour ? Or do I
have to shut down/kill every VM in the stop target in the init script ?
Thank you,
--
Laurent Léonard
14 years, 9 months
[libvirt] [PATCH] Add persistence of PCI addresses to QEMU
by Daniel P. Berrange
Current PCI addresses are allocated at time of VM startup.
To make them truely persistent, it is neccessary to do this
at time of virDomainDefine/virDomainCreate. The code in
qemuStartVMDaemon still remains in order to cope with upgrades
from older libvirt releases
* src/qemu/qemu_driver.c: Rename existing qemuAssignPCIAddresses
to qemuDetectPCIAddresses. Add new qemuAssignPCIAddresses which
does auto-allocation upfront. Call qemuAssignPCIAddresses from
qemuDomainDefine and qemuDomainCreate to assign PCI addresses that
can then be persisted. Don't clear PCI addresses at shutdown if
they are intended to be persistent
---
src/qemu/qemu_driver.c | 84 +++++++++++++++++++++++++++++++++++++++++++++---
1 files changed, 79 insertions(+), 5 deletions(-)
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index 03d0f5f..69187fc 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -95,6 +95,7 @@ struct _qemuDomainObjPrivate {
int *vcpupids;
qemuDomainPCIAddressSetPtr pciaddrs;
+ int persistentAddrs;
};
static int qemudShutdown(void);
@@ -857,6 +858,7 @@ qemuReconnectDomain(void *payload, const char *name ATTRIBUTE_UNUSED, void *opaq
virDomainObjPtr obj = payload;
struct qemud_driver *driver = opaque;
qemuDomainObjPrivatePtr priv;
+ unsigned long long qemuCmdFlags;
virDomainObjLock(obj);
@@ -872,6 +874,15 @@ qemuReconnectDomain(void *payload, const char *name ATTRIBUTE_UNUSED, void *opaq
goto error;
}
+ /* XXX we should be persisting the original flags in the XML
+ * not re-detecting them, since the binary may have changed
+ * since launch time */
+ if (qemudExtractVersionInfo(obj->def->emulator,
+ NULL,
+ &qemuCmdFlags) >= 0 &&
+ (qemuCmdFlags & QEMUD_CMD_FLAG_DEVICE))
+ priv->persistentAddrs = 1;
+
if (!(priv->pciaddrs = qemuDomainPCIAddressSetCreate(obj->def)))
goto error;
@@ -1976,7 +1987,7 @@ qemuGetPCIWatchdogVendorProduct(virDomainWatchdogDefPtr def,
* some static addrs on CLI. Have to check that...
*/
static int
-qemuAssignPCIAddresses(virDomainObjPtr vm,
+qemuDetectPCIAddresses(virDomainObjPtr vm,
qemuMonitorPCIAddress *addrs,
int naddrs)
{
@@ -2098,7 +2109,7 @@ qemuInitPCIAddresses(struct qemud_driver *driver,
&addrs);
qemuDomainObjExitMonitorWithDriver(driver, vm);
- ret = qemuAssignPCIAddresses(vm, addrs, naddrs);
+ ret = qemuDetectPCIAddresses(vm, addrs, naddrs);
VIR_FREE(addrs);
@@ -2141,6 +2152,44 @@ static int qemudNextFreeVNCPort(struct qemud_driver *driver ATTRIBUTE_UNUSED) {
return -1;
}
+
+static int
+qemuAssignPCIAddresses(virDomainDefPtr def)
+{
+ int ret = -1;
+ unsigned long long qemuCmdFlags = 0;
+ qemuDomainPCIAddressSetPtr addrs = NULL;
+ struct stat sb;
+
+ if (stat(def->emulator, &sb) < 0) {
+ virReportSystemError(errno,
+ _("Cannot find QEMU binary %s"),
+ def->emulator);
+ goto cleanup;
+ }
+
+ if (qemudExtractVersionInfo(def->emulator,
+ NULL,
+ &qemuCmdFlags) < 0)
+ goto cleanup;
+
+ if (qemuCmdFlags & QEMUD_CMD_FLAG_DEVICE) {
+ if (!(addrs = qemuDomainPCIAddressSetCreate(def)))
+ goto cleanup;
+
+ if (qemuAssignDevicePCISlots(def, addrs) < 0)
+ goto cleanup;
+ }
+
+ ret = 0;
+
+cleanup:
+ qemuDomainPCIAddressSetFree(addrs);
+
+ return ret;
+}
+
+
static pciDeviceList *
qemuGetPciHostDeviceList(virDomainDefPtr def)
{
@@ -2662,7 +2711,15 @@ static int qemudStartVMDaemon(virConnectPtr conn,
goto cleanup;
}
+ /*
+ * Normally PCI addresses are assigned inhe virDomainCreate
+ * or virDomainDefine methods. We might still need to assign
+ * some here to cope with the question of upgrades. Regardless
+ * we also need to populate the PCi address set cache for later
+ * use in hotplug
+ */
if (qemuCmdFlags & QEMUD_CMD_FLAG_DEVICE) {
+ /* Populate cache with current addresses */
if (priv->pciaddrs) {
qemuDomainPCIAddressSetFree(priv->pciaddrs);
priv->pciaddrs = NULL;
@@ -2670,8 +2727,14 @@ static int qemudStartVMDaemon(virConnectPtr conn,
if (!(priv->pciaddrs = qemuDomainPCIAddressSetCreate(vm->def)))
goto cleanup;
+
+ /* Assign any remaining addresses */
if (qemuAssignDevicePCISlots(vm->def, priv->pciaddrs) < 0)
goto cleanup;
+
+ priv->persistentAddrs = 1;
+ } else {
+ priv->persistentAddrs = 0;
}
vm->def->id = driver->nextvmid++;
@@ -2903,10 +2966,12 @@ static void qemudShutdownVMDaemon(struct qemud_driver *driver,
VIR_FREE(vm->def->seclabel.imagelabel);
}
- virDomainDefClearPCIAddresses(vm->def);
virDomainDefClearDeviceAliases(vm->def);
- qemuDomainPCIAddressSetFree(priv->pciaddrs);
- priv->pciaddrs = NULL;
+ if (!priv->persistentAddrs) {
+ virDomainDefClearPCIAddresses(vm->def);
+ qemuDomainPCIAddressSetFree(priv->pciaddrs);
+ priv->pciaddrs = NULL;
+ }
qemuDomainReAttachHostDevices(driver, vm->def);
@@ -3352,6 +3417,12 @@ static virDomainPtr qemudDomainCreate(virConnectPtr conn, const char *xml,
if (virDomainObjIsDuplicate(&driver->domains, def, 1) < 0)
goto cleanup;
+ if (qemudCanonicalizeMachine(driver, def) < 0)
+ goto cleanup;
+
+ if (qemuAssignPCIAddresses(def) < 0)
+ goto cleanup;
+
if (!(vm = virDomainAssignDef(driver->caps,
&driver->domains,
def)))
@@ -5049,6 +5120,9 @@ static virDomainPtr qemudDomainDefine(virConnectPtr conn, const char *xml) {
if (qemudCanonicalizeMachine(driver, def) < 0)
goto cleanup;
+ if (qemuAssignPCIAddresses(def) < 0)
+ goto cleanup;
+
if (!(vm = virDomainAssignDef(driver->caps,
&driver->domains,
def))) {
--
1.6.5.2
14 years, 9 months
[libvirt] [PATCH] Fix compliation of AppArmor related code
by Matthias Bolte
Broken by the latest commits to remove the virConnectPtr parameter
from internal functions.
I just pushed this one, as it is an obvious fix.
---
src/security/security_apparmor.c | 4 ++--
src/security/virt-aa-helper.c | 8 +++-----
2 files changed, 5 insertions(+), 7 deletions(-)
diff --git a/src/security/security_apparmor.c b/src/security/security_apparmor.c
index 23f40f8..db04d5c 100644
--- a/src/security/security_apparmor.c
+++ b/src/security/security_apparmor.c
@@ -262,7 +262,7 @@ use_apparmor(void)
char *libvirt_daemon = NULL;
if (virFileResolveLink("/proc/self/exe", &libvirt_daemon) < 0) {
- virSecurityReportError(NULL, VIR_ERR_INTERNAL_ERROR,
+ virSecurityReportError(VIR_ERR_INTERNAL_ERROR,
"%s", _("could not find libvirtd"));
return rc;
}
@@ -295,7 +295,7 @@ AppArmorSecurityDriverProbe(void)
}
if (!virFileExists(template)) {
- virSecurityReportError(NULL, VIR_ERR_INTERNAL_ERROR,
+ virSecurityReportError(VIR_ERR_INTERNAL_ERROR,
_("template \'%s\' does not exist"), template);
goto clean;
}
diff --git a/src/security/virt-aa-helper.c b/src/security/virt-aa-helper.c
index 619c8c3..066e18b 100644
--- a/src/security/virt-aa-helper.c
+++ b/src/security/virt-aa-helper.c
@@ -690,7 +690,7 @@ get_definition(vahControl * ctl, const char *xmlStr)
goto exit;
}
- ctl->def = virDomainDefParseString(NULL, ctl->caps, xmlStr, 0);
+ ctl->def = virDomainDefParseString(ctl->caps, xmlStr, 0);
if (ctl->def == NULL) {
vah_error(ctl, 0, "could not parse XML");
goto exit;
@@ -767,8 +767,7 @@ vah_add_file(virBufferPtr buf, const char *path, const char *perms)
}
static int
-file_iterate_cb(virConnectPtr conn ATTRIBUTE_UNUSED,
- usbDevice *dev ATTRIBUTE_UNUSED,
+file_iterate_cb(usbDevice *dev ATTRIBUTE_UNUSED,
const char *file, void *opaque)
{
virBufferPtr buf = opaque;
@@ -844,8 +843,7 @@ get_files(vahControl * ctl)
if (usb == NULL)
continue;
- rc = usbDeviceFileIterate(NULL, usb,
- file_iterate_cb, &buf);
+ rc = usbDeviceFileIterate(usb, file_iterate_cb, &buf);
usbFreeDevice(usb);
if (rc != 0)
goto clean;
--
1.6.3.3
14 years, 9 months
[libvirt] dname parameter ignored in qemudDomainMigratePrepareTunnel{, 2}
by Jim Meyering
clang warned about a dead-store in src/qemu/qemu_driver.c,
since dname is never used thereafter:
http://libvirt.org/git/?p=libvirt.git;a=blob;f=src/qemu/qemu_driver.c;h=0...
/* Target domain name, maybe renamed. */
dname = dname ? dname : def->name;
That stmt was initially added with a following use,
if (!vm) vm = virDomainFindByName(&driver->domains, dname);
But the use was removed by this commit, rendering the store "dead",
and the dname parameter effectively unused:
http://libvirt.org/git/?p=libvirt.git;a=commitdiff;h=c26cb9234f4b9fa46d7c...
The same thing happened in both
qemudDomainMigratePrepareTunnel and
qemudDomainMigratePrepareTunnel2
Shouldn't they be doing something like this instead?
if (dname)
def->name = dname;
But def->name is already allocated. Better free it first.
And is dname "known" to be allocated from the heap?
That's not clear from the little documentation I've read so far,
so I've strdup'd it below:
Here's a possible patch:
>From b51a39aed62ee5c8db733cecfe6eef0c34a7f989 Mon Sep 17 00:00:00 2001
From: Jim Meyering <meyering(a)redhat.com>
Date: Thu, 11 Feb 2010 16:46:21 +0100
Subject: [PATCH] qemu_driver.c: honor dname parameter once again
Since c26cb9234f4b9fa46d7caa3385ae36704167c53f, the dname
parameter has been ignored by these two functions. Use it.
* src/qemu/qemu_driver.c (qemudDomainMigratePrepareTunnel): Honor dname
parameter once again.
(qemudDomainMigratePrepare2): Likewise.
---
src/qemu/qemu_driver.c | 14 ++++++++++++--
1 files changed, 12 insertions(+), 2 deletions(-)
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index 0d77d57..9ff712c 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -7434,7 +7434,12 @@ qemudDomainMigratePrepareTunnel(virConnectPtr dconn,
}
/* Target domain name, maybe renamed. */
- dname = dname ? dname : def->name;
+ if (dname) {
+ VIR_FREE(def->name);
+ def->name = strdup(dname);
+ if (def->name == NULL)
+ goto cleanup;
+ }
if (virDomainObjIsDuplicate(&driver->domains, def, 1) < 0)
goto cleanup;
@@ -7660,7 +7665,12 @@ qemudDomainMigratePrepare2 (virConnectPtr dconn,
}
/* Target domain name, maybe renamed. */
- dname = dname ? dname : def->name;
+ if (dname) {
+ VIR_FREE(def->name);
+ def->name = strdup(dname);
+ if (def->name == NULL)
+ goto cleanup;
+ }
if (virDomainObjIsDuplicate(&driver->domains, def, 1) < 0)
goto cleanup;
--
1.7.0.rc2.170.gbc565
14 years, 9 months
