April 2015 - Devel - libvirt List Archives

[libvirt] [PATCH] qemu: monitor: Add check that monitor is non-null to specific commands
by Peter Krempa 15 Apr '15

15 Apr '15

Our monitor locking and cleanup code is weird as it allows to remove the monitor object pointer while we are in the monitor. Additionally EVERY api is using the vm private data after @vm was unlocked. This leads to an unpleasant situation, where when qemu crashes during a monitor session while a command is executed (and locks are down), the priv->mon pointer gets cleared (@vm is unlocked). The next monitor call then crashes on commands that did not check if the monitor is non-NULL. Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1209813 and few other possible crashes --- Since this is a workaround rather than a proper fix I'd normally not go this way, but untangling the monitor mess won't be easy so I'll fix the symptoms first. src/qemu/qemu_monitor.c | 42 ++++++++++++++++++++++++++++++++++++++++++ src/qemu/qemu_monitor.h | 12 +++++------- 2 files changed, 47 insertions(+), 7 deletions(-) diff --git a/src/qemu/qemu_monitor.c b/src/qemu/qemu_monitor.c index 2b0e1a5..e8403be 100644 --- a/src/qemu/qemu_monitor.c +++ b/src/qemu/qemu_monitor.c @@ -1171,6 +1171,12 @@ qemuMonitorUpdateVideoMemorySize(qemuMonitorPtr mon, int ret = -1; char *path = NULL; + if (!mon) { + virReportError(VIR_ERR_INVALID_ARG, "%s", + _("monitor must not be NULL")); + return -1; + } + if (mon->json) { ret = qemuMonitorFindObjectPath(mon, "/", videoName, &path); if (ret < 0) { @@ -1886,6 +1892,12 @@ qemuMonitorBlockStatsUpdateCapacity(qemuMonitorPtr mon, { VIR_DEBUG("mon=%p, stats=%p, backing=%d", mon, stats, backingChain); + if (!mon) { + virReportError(VIR_ERR_INVALID_ARG, "%s", + _("monitor must not be NULL")); + return -1; + } + if (!mon->json) { virReportError(VIR_ERR_OPERATION_UNSUPPORTED, "%s", _("block capacity/size info requires JSON monitor")); @@ -3246,6 +3258,12 @@ qemuMonitorAddObject(qemuMonitorPtr mon, mon, type, objalias, props); int ret = -1; + if (!mon) { + virReportError(VIR_ERR_INVALID_ARG, "%s", + _("monitor must not be NULL")); + return -1; + } + if (mon->json) { ret = qemuMonitorJSONAddObject(mon, type, objalias, props); } else { @@ -3265,6 +3283,12 @@ qemuMonitorDelObject(qemuMonitorPtr mon, VIR_DEBUG("mon=%p objalias=%s", mon, objalias); int ret = -1; + if (!mon) { + virReportError(VIR_ERR_INVALID_ARG, "%s", + _("monitor must not be NULL")); + return -1; + } + if (mon->json) ret = qemuMonitorJSONDelObject(mon, objalias); else @@ -3453,6 +3477,12 @@ qemuMonitorBlockCommit(qemuMonitorPtr mon, const char *device, "bandwidth=%llu", mon, device, top, base, NULLSTR(backingName), bandwidth); + if (!mon) { + virReportError(VIR_ERR_INVALID_ARG, "%s", + _("monitor must not be NULL")); + return -1; + } + if (mon->json) ret = qemuMonitorJSONBlockCommit(mon, device, top, base, backingName, bandwidth); @@ -3482,6 +3512,12 @@ qemuMonitorDiskNameLookup(qemuMonitorPtr mon, virStorageSourcePtr top, virStorageSourcePtr target) { + if (!mon) { + virReportError(VIR_ERR_INVALID_ARG, "%s", + _("monitor must not be NULL")); + return NULL; + } + if (!mon->json) { virReportError(VIR_ERR_OPERATION_UNSUPPORTED, "%s", _("JSON monitor is required")); @@ -3592,6 +3628,12 @@ qemuMonitorBlockJob(qemuMonitorPtr mon, mon, device, NULLSTR(base), NULLSTR(backingName), bandwidth, mode, modern); + if (!mon) { + virReportError(VIR_ERR_INVALID_ARG, "%s", + _("monitor must not be NULL")); + return -1; + } + if (mon->json) ret = qemuMonitorJSONBlockJob(mon, device, base, backingName, bandwidth, mode, modern); diff --git a/src/qemu/qemu_monitor.h b/src/qemu/qemu_monitor.h index a3514cf..7e62139 100644 --- a/src/qemu/qemu_monitor.h +++ b/src/qemu/qemu_monitor.h @@ -246,7 +246,7 @@ void qemuMonitorSetOptions(qemuMonitorPtr mon, virJSONValuePtr options) int qemuMonitorUpdateVideoMemorySize(qemuMonitorPtr mon, virDomainVideoDefPtr video, const char *videName) - ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2) ATTRIBUTE_NONNULL(3); + ATTRIBUTE_NONNULL(2) ATTRIBUTE_NONNULL(3); int qemuMonitorHMPCommandWithFd(qemuMonitorPtr mon, const char *cmd, int scm_fd, @@ -381,7 +381,7 @@ int qemuMonitorGetAllBlockStatsInfo(qemuMonitorPtr mon, int qemuMonitorBlockStatsUpdateCapacity(qemuMonitorPtr mon, virHashTablePtr stats, bool backingChain) - ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2); + ATTRIBUTE_NONNULL(2); int qemuMonitorGetBlockExtent(qemuMonitorPtr mon, const char *dev_name, @@ -730,15 +730,13 @@ int qemuMonitorBlockCommit(qemuMonitorPtr mon, const char *base, const char *backingName, unsigned long long bandwidth) - ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2) ATTRIBUTE_NONNULL(3) - ATTRIBUTE_NONNULL(4); + ATTRIBUTE_NONNULL(2) ATTRIBUTE_NONNULL(3) ATTRIBUTE_NONNULL(4); bool qemuMonitorSupportsActiveCommit(qemuMonitorPtr mon); char *qemuMonitorDiskNameLookup(qemuMonitorPtr mon, const char *device, virStorageSourcePtr top, virStorageSourcePtr target) - ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2) ATTRIBUTE_NONNULL(3) - ATTRIBUTE_NONNULL(4); + ATTRIBUTE_NONNULL(2) ATTRIBUTE_NONNULL(3) ATTRIBUTE_NONNULL(4); int qemuMonitorArbitraryCommand(qemuMonitorPtr mon, const char *cmd, @@ -774,7 +772,7 @@ int qemuMonitorBlockJobInfo(qemuMonitorPtr mon, const char *device, virDomainBlockJobInfoPtr info, unsigned long long *bandwidth) - ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2) ATTRIBUTE_NONNULL(3); + ATTRIBUTE_NONNULL(2) ATTRIBUTE_NONNULL(3); int qemuMonitorOpenGraphics(qemuMonitorPtr mon, const char *protocol, -- 2.3.5

2 2

[libvirt] [PATCH] sanlock: Use VIR_ERR_RESOURCE_BUSY if sanlock_acquire fails
by Jiri Denemark 15 Apr '15

15 Apr '15

When acquiring resource via sanlock fails, we would report it as VIR_ERR_INTERNAL_ERROR, which is not very friendly to applications using libvirt. Moreover, the lockd driver would report the same failure as VIR_ERR_RESOURCE_BUSY, which looks better. Unfortunately, in sanlock driver we don't really know if acquiring the resource failed because it was already locked or there was another reason behind. But the end result is the same and I think using VIR_ERR_RESOURCE_BUSY reason for all acquire failures is still better than what we have now. https://bugzilla.redhat.com/show_bug.cgi?id=1165119 Signed-off-by: Jiri Denemark <jdenemar(a)redhat.com> --- src/locking/lock_driver_sanlock.c | 2 +- src/util/virerror.c | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/src/locking/lock_driver_sanlock.c b/src/locking/lock_driver_sanlock.c index dbe79bc..e052875 100644 --- a/src/locking/lock_driver_sanlock.c +++ b/src/locking/lock_driver_sanlock.c @@ -975,7 +975,7 @@ static int virLockManagerSanlockAcquire(virLockManagerPtr lock, priv->res_count, priv->res_args, opt)) < 0) { if (rv <= -200) - virReportError(VIR_ERR_INTERNAL_ERROR, + virReportError(VIR_ERR_RESOURCE_BUSY, _("Failed to acquire lock: error %d"), rv); else virReportSystemError(-rv, "%s", diff --git a/src/util/virerror.c b/src/util/virerror.c index c4e84e7..91c8686 100644 --- a/src/util/virerror.c +++ b/src/util/virerror.c @@ -1326,7 +1326,7 @@ virErrorMsg(virErrorNumber error, const char *info) if (info == NULL) errmsg = _("resource busy"); else - errmsg = _("resource busy %s"); + errmsg = _("resource busy: %s"); break; case VIR_ERR_ACCESS_DENIED: if (info == NULL) -- 2.3.5

2 1

[libvirt] [PATCH] Add articles to virDomainDeviceDetachFlags docs
by Ján Tomko 14 Apr '15

14 Apr '15

Reported by John Ferlan. --- Pushed as trivial. src/libvirt-domain.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/libvirt-domain.c b/src/libvirt-domain.c index 0acfd13..ccacdb4 100644 --- a/src/libvirt-domain.c +++ b/src/libvirt-domain.c @@ -8463,7 +8463,7 @@ virDomainDetachDevice(virDomainPtr domain, const char *xml) * configuration before it was actually removed by the hypervisor causing * various failures on subsequent operations. To check whether the device was * successfully removed, either recheck domain configuration using - * virDomainGetXMLDesc() or add handler for VIR_DOMAIN_EVENT_ID_DEVICE_REMOVED + * virDomainGetXMLDesc() or add a handler for the VIR_DOMAIN_EVENT_ID_DEVICE_REMOVED * event. In case the device is already gone when virDomainDetachDeviceFlags * returns, the event is delivered before this API call ends. To help existing * clients work better in most cases, this API will try to transform an -- 2.0.5

1 0

[libvirt] [PATCH] xen: fix build error on rhel-5
by Pavel Hrdina 14 Apr '15

14 Apr '15

../../src/xen/block_stats.c:82: warning: dereferencing type-punned pointer will break strict-aliasing rules [-Wstrict-aliasing] Signed-off-by: Pavel Hrdina <phrdina(a)redhat.com> --- src/xen/block_stats.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/xen/block_stats.c b/src/xen/block_stats.c index e1e10a2..9dd3a07 100644 --- a/src/xen/block_stats.c +++ b/src/xen/block_stats.c @@ -63,7 +63,7 @@ static int64_t read_stat(const char *path) { char str[64]; - int64_t r; + long long r; size_t i; FILE *fp; @@ -79,10 +79,10 @@ read_stat(const char *path) return -1; str[i] = '\0'; /* make sure the string is nul-terminated */ - if (virStrToLong_ll(str, NULL, 10, (long long *) &r) < 0) + if (virStrToLong_ll(str, NULL, 10, &r) < 0) return -1; - return r; + return (int64_t)r; } static int64_t -- 2.0.5

2 2

[libvirt] [PATCH] hostdev: fix loop index error when resetvfnetconfig
by Huanle Han 14 Apr '15

14 Apr '15

The variable 'last_processed_hostdev_vf' indicates index of the last successfully configed vf. When resetvfnetconfig because of failure, hostdevs[last_processed_hostdev_vf] should also be reset. Signed-off-by: Huanle Han <hanxueluo(a)gmail.com> --- src/util/virhostdev.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/util/virhostdev.c b/src/util/virhostdev.c index 83f567d..b15db70 100644 --- a/src/util/virhostdev.c +++ b/src/util/virhostdev.c @@ -693,7 +693,7 @@ virHostdevPreparePCIDevices(virHostdevManagerPtr hostdev_mgr, resetvfnetconfig: for (i = 0; - last_processed_hostdev_vf != -1 && i < last_processed_hostdev_vf; i++) + last_processed_hostdev_vf != -1 && i <= last_processed_hostdev_vf; i++) virHostdevNetConfigRestore(hostdevs[i], hostdev_mgr->stateDir, NULL); reattachdevs: -- 1.9.1

2 1

[libvirt] [PATCH] qemu: fix index error when clean up vport profile
by Huanle Han 14 Apr '15

14 Apr '15

1. 'last_good_net' indicates the index of last successfully configured net. so def->nets[last_good_net] should also be clean up if error occurs. 2. if error occurs in 'virNetDevMacVLanVPortProfileRegisterCallback' (second 'goto err_exit' in loop), we should also do 'virNetDevVPortProfileDisassociate' cleanup for the 'virNetDevVPortProfileAssociate'(first code block in loop). So we should consider the net is successfully configured after first code block in loop finishes. Signed-off-by: Huanle Han <hanxueluo(a)gmail.com> --- src/qemu/qemu_migration.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/qemu/qemu_migration.c b/src/qemu/qemu_migration.c index 8c45415..e214735 100644 --- a/src/qemu/qemu_migration.c +++ b/src/qemu/qemu_migration.c @@ -5067,6 +5067,7 @@ qemuMigrationVPAssociatePortProfiles(virDomainDefPtr def) net->ifname); goto err_exit; } + last_good_net = i; VIR_DEBUG("Port profile Associate succeeded for %s", net->ifname); if (virNetDevMacVLanVPortProfileRegisterCallback(net->ifname, &net->mac, @@ -5075,13 +5076,12 @@ qemuMigrationVPAssociatePortProfiles(virDomainDefPtr def) VIR_NETDEV_VPORT_PROFILE_OP_CREATE)) goto err_exit; } - last_good_net = i; } return 0; err_exit: - for (i = 0; last_good_net != -1 && i < last_good_net; i++) { + for (i = 0; last_good_net != -1 && i <= last_good_net; i++) { net = def->nets[i]; if (virDomainNetGetActualType(net) == VIR_DOMAIN_NET_TYPE_DIRECT) { ignore_value(virNetDevVPortProfileDisassociate(net->ifname, -- 1.9.1

2 1

[libvirt] [PATCH 0/5] Prohibit use of 'index' for variable
by John Ferlan 14 Apr '15

14 Apr '15

Given the recent Xen-devel build failure/issue - this series of patches will remove our uses of '[unsigned] int index' and then generate a rule for it John Ferlan (5): qemu: Adjust the prototype to match the function util: Use 'idx' instead of 'index' snapshot: Change 'index' to 'idx' nwfilter: Change 'index' to 'idx' cfg: Disallow usage of 'index' as variable name cfg.mk | 5 +++++ src/conf/nwfilter_params.c | 8 ++++---- src/conf/nwfilter_params.h | 2 +- src/conf/snapshot_conf.c | 6 +++--- src/conf/snapshot_conf.h | 2 +- src/qemu/qemu_conf.h | 2 +- src/qemu/qemu_driver.c | 4 ++-- src/util/virnetdev.c | 4 ++-- 8 files changed, 19 insertions(+), 14 deletions(-) -- 2.1.0

2 6

[libvirt] [PATCH 1/1] virt-aa-helper: add unix channels (esp for qemu-guest-agent)
by Serge Hallyn 14 Apr '15

14 Apr '15

The original bug report was at https://bugs.launchpad.net/ubuntu/+source/libvirt/+bug/1393842 Signed-off-by: Serge Hallyn <serge.hallyn(a)ubuntu.com> --- src/security/virt-aa-helper.c | 1 + 1 file changed, 1 insertion(+) diff --git a/src/security/virt-aa-helper.c b/src/security/virt-aa-helper.c index e53779e..8ec95c1 100644 --- a/src/security/virt-aa-helper.c +++ b/src/security/virt-aa-helper.c @@ -998,6 +998,7 @@ get_files(vahControl * ctl) (ctl->def->channels[i]->source.type == VIR_DOMAIN_CHR_TYPE_PTY || ctl->def->channels[i]->source.type == VIR_DOMAIN_CHR_TYPE_DEV || ctl->def->channels[i]->source.type == VIR_DOMAIN_CHR_TYPE_FILE || + ctl->def->channels[i]->source.type == VIR_DOMAIN_CHR_TYPE_UNIX || ctl->def->channels[i]->source.type == VIR_DOMAIN_CHR_TYPE_PIPE) && ctl->def->channels[i]->source.data.file.path) if (vah_add_file_chardev(&buf, -- 2.1.4

2 5

[libvirt] [PATCH] sparc: Add default PCI root controller
by Martin Kletzander 14 Apr '15

14 Apr '15

It is there even with -nodefaults and -no-user-config, so count with that so we can start sparc domains. Signed-off-by: Martin Kletzander <mkletzan(a)redhat.com> --- src/qemu/qemu_domain.c | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c index ae632c5..603360f 100644 --- a/src/qemu/qemu_domain.c +++ b/src/qemu/qemu_domain.c @@ -1,7 +1,7 @@ /* * qemu_domain.c: QEMU domain private state * - * Copyright (C) 2006-2014 Red Hat, Inc. + * Copyright (C) 2006-2015 Red Hat, Inc. * Copyright (C) 2006 Daniel P. Berrange * * This library is free software; you can redistribute it and/or @@ -999,6 +999,12 @@ qemuDomainDefPostParse(virDomainDefPtr def, case VIR_ARCH_S390X: addDefaultUSB = false; break; + + case VIR_ARCH_SPARC: + case VIR_ARCH_SPARC64: + addPCIRoot = true; + break; + default: break; } -- 2.3.5

2 1

[libvirt] [PATCH] PowerPC: Replace hardcoded values of 'pseries-2*' machine in schema
by Prerna Saxena 14 Apr '15

14 Apr '15

From: Prerna Saxena <prerna(a)linux.vnet.ibm.com> Date: Mon, 6 Apr 2015 22:56:25 -0500 Qemu-system-ppc64 has introduced new variants of pseries machine type, such as 'pseries', 'pseries-2.1'..'pseries-2.3'. Replace the hardcoded values in the schema with a regex that captures this. Signed-off-by: Prerna Saxena <prerna(a)linux.vnet.ibm.com> --- docs/schemas/domaincommon.rng | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/docs/schemas/domaincommon.rng b/docs/schemas/domaincommon.rng index 7406d92..a192637 100644 --- a/docs/schemas/domaincommon.rng +++ b/docs/schemas/domaincommon.rng @@ -421,9 +421,9 @@ <optional> <attribute name="machine"> <choice> - <value>pseries</value> - <value>pseries-2.1</value> - <value>pseries-2.2</value> + <data type="string"> + <param name="pattern">(pseries)(\-2\.[1-9])?</param> + </data> </choice> </attribute> </optional> -- 1.8.3.1 -- Prerna Saxena Linux Technology Centre, IBM Systems and Technology Lab, Bangalore, India

2 1