April 2015 - Devel - libvirt List Archives

[libvirt] [PATCH] virNetSocketNewConnectUNIX: Use flocks when spawning a daemon
by Michal Privoznik 02 Apr '15

02 Apr '15

https://bugzilla.redhat.com/show_bug.cgi?id=1200149 Even though we have a mutex mechanism so that two clients don't spawn two daemons, it's not strong enough. It can happen that while one client is spawning the daemon, the other one fails to connect. Basically two possible errors can happen: error: Failed to connect socket to '/home/mprivozn/.cache/libvirt/libvirt-sock': Connection refused or: error: Failed to connect socket to '/home/mprivozn/.cache/libvirt/libvirt-sock': No such file or directory The problem in both cases is, the daemon is only starting up, while we are trying to connect (and fail). We should postpone the connecting phase until the daemon is started (by the other thread that is spawning it). In order to do that, create a file lock 'libvirt-lock' in the directory where session daemon would create its socket. So even when called from multiple processes, spawning a daemon will serialize on the file lock. So only the first to come will spawn the daemon. Signed-off-by: Michal Privoznik <mprivozn(a)redhat.com> --- src/rpc/virnetsocket.c | 46 ++++++++++++++++++++++++++++++---------------- 1 file changed, 30 insertions(+), 16 deletions(-) diff --git a/src/rpc/virnetsocket.c b/src/rpc/virnetsocket.c index 6b019cc..8d8b6e0 100644 --- a/src/rpc/virnetsocket.c +++ b/src/rpc/virnetsocket.c @@ -545,8 +545,10 @@ int virNetSocketNewConnectUNIX(const char *path, { char *binname = NULL; char *pidpath = NULL; + char *lockpath = NULL; int fd, passfd = -1; int pidfd = -1; + int lockfd = -1; virSocketAddr localAddr; virSocketAddr remoteAddr; @@ -561,6 +563,22 @@ int virNetSocketNewConnectUNIX(const char *path, return -1; } + if (spawnDaemon) { + if (virPidFileConstructPath(false, NULL, "libvirt-lock", &lockpath) < 0) + goto error; + + if ((lockfd = open(lockpath, O_RDWR | O_CREAT, 0600)) < 0 || + virSetCloseExec(lockfd) < 0) { + virReportSystemError(errno, _("Unable to create lock '%s'"), lockpath); + goto error; + } + + if (virFileLock(lockfd, false, 0, 1, true) < 0) { + virReportSystemError(errno, _("Unable to lock '%s'"), lockpath); + goto error; + } + } + if ((fd = socket(PF_UNIX, SOCK_STREAM, 0)) < 0) { virReportSystemError(errno, "%s", _("Failed to create socket")); goto error; @@ -595,17 +613,8 @@ int virNetSocketNewConnectUNIX(const char *path, if (virPidFileConstructPath(false, NULL, binname, &pidpath) < 0) goto error; - pidfd = virPidFileAcquirePath(pidpath, false, getpid()); - if (pidfd < 0) { - /* - * This can happen in a very rare case of two clients spawning two - * daemons at the same time, and the error in the logs that gets - * reset here can be a clue to some future debugging. - */ - virResetLastError(); - spawnDaemon = false; - goto retry; - } + if ((pidfd = virPidFileAcquirePath(pidpath, false, getpid())) < 0) + goto error; if ((passfd = socket(PF_UNIX, SOCK_STREAM, 0)) < 0) { virReportSystemError(errno, "%s", _("Failed to create socket")); @@ -668,16 +677,17 @@ int virNetSocketNewConnectUNIX(const char *path, goto error; } - /* - * Do we need to eliminate the super-rare race here any more? It would - * need incorporating the following VIR_FORCE_CLOSE() into a - * virCommandHook inside a virNetSocketForkDaemon(). - */ VIR_FORCE_CLOSE(pidfd); if (virNetSocketForkDaemon(binary, passfd) < 0) goto error; } + if (lockfd) { + unlink(lockpath); + VIR_FORCE_CLOSE(lockfd); + VIR_FREE(lockpath); + } + localAddr.len = sizeof(localAddr.data); if (getsockname(fd, &localAddr.data.sa, &localAddr.len) < 0) { virReportSystemError(errno, "%s", _("Unable to get local socket name")); @@ -694,10 +704,14 @@ int virNetSocketNewConnectUNIX(const char *path, error: if (pidfd >= 0) virPidFileDeletePath(pidpath); + if (lockfd) + unlink(lockpath); VIR_FREE(pidpath); + VIR_FREE(lockpath); VIR_FORCE_CLOSE(fd); VIR_FORCE_CLOSE(passfd); VIR_FORCE_CLOSE(pidfd); + VIR_FORCE_CLOSE(lockfd); if (spawnDaemon) unlink(path); return -1; -- 2.0.5

2 2

Re: [libvirt] [Qemu-devel] [PATCH v4 11/15] target-s390x: New QMP command query-cpu-model
by Eduardo Habkost 02 Apr '15

02 Apr '15

(CCing libvir-list and Jiri Denemark for libvirt-related discussion about -cpu host/none, and live-migration safety expectations) On Wed, Apr 01, 2015 at 06:31:23PM +0200, Michael Mueller wrote: > On Wed, 1 Apr 2015 10:01:13 -0300 > Eduardo Habkost <ehabkost(a)redhat.com> wrote: > > > On Tue, Mar 31, 2015 at 10:09:09PM +0200, Michael Mueller wrote: > > > On Tue, 31 Mar 2015 15:35:26 -0300 > > > Eduardo Habkost <ehabkost(a)redhat.com> wrote: > > > > > > > On Mon, Mar 30, 2015 at 04:28:24PM +0200, Michael Mueller wrote: > > > > > This patch implements a new QMP request named 'query-cpu-model'. > > > > > It returns the cpu model of cpu 0 and its backing accelerator. > > > > > > > > > > request: > > > > > {"execute" : "query-cpu-model" } > > > > > > > > > > answer: > > > > > {"return" : {"name": "2827-ga2", "accel": "kvm" }} > > > > > > > > > > Alias names are resolved to their respective machine type and GA names > > > > > already during cpu instantiation. Thus, also a cpu model like 'host' > > > > > which is implemented as alias will return its normalized cpu model name. > > > > > > > > > > Furthermore the patch implements the following function: > > > > > > > > > > - s390_cpu_models_used(), returns true if S390 cpu models are in use > > > > > > > > > > Signed-off-by: Michael Mueller <mimu(a)linux.vnet.ibm.com> > > > > > --- > > > > [...] > > > > > +static inline char *strdup_s390_cpu_name(S390CPUClass *cc) > > > > > +{ > > > > > + return g_strdup_printf("%04x-ga%u", cc->proc.type, cc->mach.ga); > > > > > +} > > > > > > > > How exactly is this information going to be used by clients? If getting > > > > the correct type and ga values is important for them, maybe you could > > > > add them as integer fields, instead of requiring clients to parse the > > > > CPU model name? > > > > > > The consumer don't need to parse the name, it is just important for them to have > > > distinctive names that correlate with the names returned by query-cpu-definitions. > > > Once the name of an active guest is known, e.g. ("2827-ga2", "kvm") a potential > > > migration target can be verified, i.e. its query-cpu-definitions answer for "kvm" > > > has to contain "2827-ga2" with the attribute runnable set to true. With that mechanism > > > also the largest common denominator can be calculated. That model will be used then. > > > > Understood. So the point is to really have a name that can be found at > > query-cpu-definitions. Makes sense. > > > > (BTW, if you reused strdup_s390_cpu_name() inside > > s390_cpu_compare_class_name() too, you would automatically ensure that > > query-cpus, query-cpu-definitions and s390_cpu_class_by_name() will > > always agree with each other). > > I have to verify but it seems to make sense from reading... I will do that if it works. :-) > > > > > > > > > I also changed the above mentioned routine to map the cpu model none case: > > > > > > static inline char *strdup_s390_cpu_name(S390CPUClass *cc) > > > { > > > if (cpuid(cc->proc)) { > > > return g_strdup_printf("%04x-ga%u", cc->proc.type, cc->mach.ga); > > > } else { > > > return g_strdup("none"); > > > } > > > } > > > > What about: > > > > static const char *s390_cpu_name(S390CPUClass *cc) > > { > > return cc->model_name; > > } > > > > And then you can just set cc->model_name=_name inside S390_PROC_DEF (and > > set it to "none" inside s390_cpu_class_init()). > > > > Wouldn't that store redundant information... but it would at least shift the work into the > initialization phase and do the format just once per model. To be honest, calculating the CPU model name on the fly with strdup_s390_cpu_name() like you did above wouldn't be a problem to me. I just wanted to avoid having the same CPU model name logic (and special cases like "none") duplicated inside strdup_s390_cpu_name(), S390_PROC_DEF, s390_cpu_class_by_name(), and maybe other places. Maybe this duplication can be eliminated by simply reusing strdup_s390_cpu_name() inside s390_cpu_compare_class_name(). > > > I wonder if this class->model_name conversion could be made generic > > inside the CPU class. We already have a CPU::class_by_name() method, so > > it makes sense to have the opposite function too. > > > > (But I wouldn't mind making this s390-specific first, and converted > > later to generic code if appropriate). > > ok > > > > > > > > > This implicitly will fail a comparison for cpu model ("none", "kvm") as that will > > > never be part of the query-cpu-definitions answer. > > > > I am not sure I follow. If ("none", "kvm") is never in the list, is > > "-cpu none -machine accel=kvm" always an invalid use case? > > Not directly invalid as "-cpu none" will be the same as omitting the -cpu option. > KVM will setup the vcpu properties withou any QEMU control to whatever the hosting > machine and the kvm kernel code offers. That will allow to run QEMU against a KVM > version that is not aware of the s390 cpu model ioctls. It looks like we have conflicting expectations about query-cpu-definitions, it seems: on the one hand, if "-cpu none" is valid I believe it should appear on the query-cpu-definitions return value; on the other hand, it is not (always?) migration-safe, so just comparing the source query-cpus data with the target query-cpu-definitions data wouldn't be enough to ensure live-migration safety. On x86, we have a similar problem with "-cpu host", that changes depending on the host hardware and host kernel. We solve this problem by making libvirt code aware of the set of valid CPU models, and libvirt has special cases for "-cpu host". If you don't want to encode that knowledge in libvirt or other management software for s390, it looks like you need something like a "stable-abi-safe" field on CpuDefinitionInfo? > > > > > (I don't understand completely the meaning of "-cpu none" yet. How does > > the CPU look like for the guest in this case? Is it possible to > > live-migrate when using -cpu none?) > > And yes, that does not make sense in a migration context. The answer on query-cpu-model > (or query-cpus) will be ("none", "kvm") and that will never match a runnable model. > The guest cpu will be derived from the hosting system and the kvm kernel as it is currently > without the cpu model interface. > > I hope I made it better to understand now... Yes, thanks! -- Eduardo

2 5

[libvirt] [PATCH 0/2] fix 2 bugs in virHostdevReAttachPCIDevices
by Huanle Han 02 Apr '15

02 Apr '15

Fix 1: Fix index error in loop after remove an element from the array 'virPCIDeviceList' is actually an array. Removing one element makes the rest of the element move. Use while loop, increase index only when not do 'virPCIDeviceListDel(pcidevs, dev)' Fix 2: In such a case: 1. Domain A and B xml contain the same SRIOV net hostdev(<interface type='hostdev' /> with same pci address). 2. virsh start A (Successfully, and configure the SRIOV net with custom mac) 3. virsh start B (Fail because of the hostdev used by domain A or other reason.) In step 3, 'virHostdevNetConfigRestore' is called for the hostdev which is still used by domain A. It makes the mac/vlan of the SRIOV net change. Code Change in this fix: 1. As the pci used by other domain have been removed from 'pcidevs' in previous loop, we only restore the nic config for the hostdev still in 'pcidevs'(used by this domain) 2. wrap a function 'virHostdevIsPCINetDevice', which detect whether the hostdev is a pci net device or not. 3. update the comments to make it more clear Huanle Han (2): hostdev: Fix index error in loop after remove an element hostdev: fix net config restore error src/util/virhostdev.c | 56 ++++++++++++++++++++++++++++++++++++--------------- 1 file changed, 40 insertions(+), 16 deletions(-) -- 1.9.1

3 9

[libvirt] [PATCH] Typos: Get rid of dependan(t|cies)
by Martin Kletzander 02 Apr '15

02 Apr '15

Dependant is flagged as wrong in US dictionary (only valid in UK dictionary, and even then, it has only the financial sense and not the inter-relatedness sense that we are more prone to be wanting throughout code). Signed-off-by: Martin Kletzander <mkletzan(a)redhat.com> --- Pushed as trivial and already decided on in here: https://www.redhat.com/archives/libvir-list/2015-March/msg01320.html src/libvirt-domain.c | 4 ++-- src/libvirt-secret.c | 2 +- src/util/virkmod.c | 2 +- tools/virsh.pod | 2 +- 4 files changed, 5 insertions(+), 5 deletions(-) diff --git a/src/libvirt-domain.c b/src/libvirt-domain.c index f1608dc..0d00af8 100644 --- a/src/libvirt-domain.c +++ b/src/libvirt-domain.c @@ -2642,7 +2642,7 @@ virDomainGetXMLDesc(virDomainPtr domain, unsigned int flags) * * Reads native configuration data describing a domain, and * generates libvirt domain XML. The format of the native - * data is hypervisor dependant. + * data is hypervisor dependent. * * Returns a 0 terminated UTF-8 encoded XML instance, or NULL in case of error. * the caller must free() the returned value. @@ -2692,7 +2692,7 @@ virConnectDomainXMLFromNative(virConnectPtr conn, * * Reads a domain XML configuration document, and generates * a native configuration file describing the domain. - * The format of the native data is hypervisor dependant. + * The format of the native data is hypervisor dependent. * * Returns a 0 terminated UTF-8 encoded native config datafile, or NULL in case of error. * the caller must free() the returned value. diff --git a/src/libvirt-secret.c b/src/libvirt-secret.c index 946d1b8..e77f223 100644 --- a/src/libvirt-secret.c +++ b/src/libvirt-secret.c @@ -452,7 +452,7 @@ virSecretGetUsageType(virSecretPtr secret) * * Get the unique identifier of the object with which this * secret is to be used. The format of the identifier is - * dependant on the usage type of the secret. For a secret + * dependent on the usage type of the secret. For a secret * with a usage type of VIR_SECRET_USAGE_TYPE_VOLUME the * identifier will be a fully qualfied path name. The * identifiers are intended to be unique within the set of diff --git a/src/util/virkmod.c b/src/util/virkmod.c index e0bcdfc..219fad6 100644 --- a/src/util/virkmod.c +++ b/src/util/virkmod.c @@ -121,7 +121,7 @@ virKModLoad(const char *module, bool useBlacklist) * Remove or unload a module. * * NB: Do not use 'modprobe -r' here as that code will recursively - * unload any modules that were dependancies of the one being removed + * unload any modules that were dependencies of the one being removed * even if things still require them. e.g. it'll see the 'bridge' * module has refcount of 0 and remove it, even if there are bridges * created on the host diff --git a/tools/virsh.pod b/tools/virsh.pod index 5d52761..79d50f9 100644 --- a/tools/virsh.pod +++ b/tools/virsh.pod @@ -3235,7 +3235,7 @@ B<Note>: A storage pool that relies on remote resources such as an times in order to have all the volumes detected (see B<pool-refresh>). This is because the corresponding volume devices may not be present in the host's filesystem during the initial pool startup or the current -refresh attempt. The number of refresh retries is dependant upon the +refresh attempt. The number of refresh retries is dependent upon the network connection and the time the host takes to export the corresponding devices. -- 2.3.4

1 0

[libvirt] [PATCH] docs: Add Host sFlow into monitoring apps
by Martin Kletzander 02 Apr '15

02 Apr '15

Reported-by: Peter Phaal <peter.phaal(a)gmail.com> Signed-off-by: Martin Kletzander <mkletzan(a)redhat.com> --- docs/apps.html.in | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/docs/apps.html.in b/docs/apps.html.in index 79ee73a..fd45189 100644 --- a/docs/apps.html.in +++ b/docs/apps.html.in @@ -321,6 +321,12 @@ For a full description, please refer to the libvirt section in the collectd.conf(5) manual page. </dd> + <dt><a href="http://host-sflow.sourceforge.net/">Host sFlow</a></dt> + <dd> + Host sFlow is a lightweight agent running on KVM hypervisors that + links to libvirt library and exports standardized cpu, memory, network + and disk metrics for all virtual machines. + </dd> <dt><a href="http://honk.sigxcpu.org/projects/libvirt/#munin">Munin</a></dt> <dd> The plugins provided by Guido Günther allow to monitor various things -- 2.3.3

2 1

[libvirt] [PATCHv3 0/6] Allocate virtio-serial addresses
by Ján Tomko 02 Apr '15

02 Apr '15

Fixes: https://bugzilla.redhat.com/show_bug.cgi?id=1076708 https://bugzilla.redhat.com/show_bug.cgi?id=890606 New in v3: * preserve the behavior of honoring the controller if a partial address is specified: <address type='virtio-serial' controller='2'/> * automatically add a controller if we're out of free ports and add a test for it Ján Tomko (6): Add test for virtio serial port assignment Add functions to track virtio-serial addresses Allocate virtio-serial addresses when starting a domain Expand the address set when attaching a virtio-serial controller Assign an address when hotplugging a virtio-serial device Auto add virtio-serial controllers src/conf/domain_addr.c | 435 +++++++++++++++++++++ src/conf/domain_addr.h | 61 +++ src/conf/domain_conf.c | 48 +-- src/conf/domain_conf.h | 1 + src/libvirt_private.syms | 11 + src/qemu/qemu_command.c | 63 +++ src/qemu/qemu_domain.c | 1 + src/qemu/qemu_domain.h | 1 + src/qemu/qemu_hotplug.c | 32 +- src/qemu/qemu_process.c | 2 + tests/qemuhotplugtest.c | 2 +- .../qemuxml2argv-channel-virtio-auto.args | 2 +- .../qemuxml2argv-channel-virtio-autoadd.args | 33 ++ .../qemuxml2argv-channel-virtio-autoadd.xml | 45 +++ .../qemuxml2argv-channel-virtio-autoassign.args | 20 + .../qemuxml2argv-channel-virtio-autoassign.xml | 50 +++ tests/qemuxml2argvtest.c | 4 + .../qemuxml2xmlout-channel-virtio-auto.xml | 9 +- 18 files changed, 777 insertions(+), 43 deletions(-) create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-channel-virtio-autoadd.args create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-channel-virtio-autoadd.xml create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-channel-virtio-autoassign.args create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-channel-virtio-autoassign.xml -- 2.0.5

2 8

[libvirt] [PATCH v2 0/6] storage: handle scsi/iscsi error paths better
by John Ferlan 02 Apr '15

02 Apr '15

v1 here: http://www.redhat.com/archives/libvir-list/2015-March/msg01562.html changes: 1/6: Removed the virGetLastError() == NULL check 2/6: New - reading more closely showed an error path without a goto 3/6: New - Adjust return for virStorageBackendSCSINewLun to be able to differentiate real error vs. deciding to return because a stable path could not be determined 4/6: New - Found an unused variable in processLU 5/6: Adjust the logic to return -1 for "real" errors and -2 for errors that are "recoverable". Removed the need for goto cleanup. 6/6: Add check for non-existent stable path at startup (and fail). Add check during refresh for a found non standard stable path that returns no volumes in the pool John Ferlan (6): iscsi: Use error message from virStorageBackendSCSIFindLUs iscsi: Fix exit path for virStorageBackendISCSIFindLUs failure scsi: Adjust return value for virStorageBackendSCSINewLun scsi: Remove unused 'type_path' in processLU scsi: Adjust return values from processLU iscsi: Add checks for non standard stable target.path src/storage/storage_backend_iscsi.c | 33 ++++++++++++++++----- src/storage/storage_backend_scsi.c | 59 ++++++++++++++++++++++++------------- 2 files changed, 64 insertions(+), 28 deletions(-) -- 2.1.0

2 12

[libvirt] [PATCH 0/7] Introduce storage pool status XML
by Erik Skultety 02 Apr '15

02 Apr '15

All of the commits resolve BZ https://bugzilla.redhat.com/show_bug.cgi?id=1177733 Erik Skultety (7): storage: Remove unused attribute conn from 'checkPool' callback conf: Add support for storage state directory conf: Add/modify storage formatting functions storage: Modify stateInitialize to support storage state XML conf: Introduce virStoragePoolLoadAllState && virStoragePoolLoadState storage: Introduce storagePoolUpdateAllState function storage: Create/Delete pool status XML src/conf/storage_conf.c | 229 ++++++++++++++++++++++++++++------ src/conf/storage_conf.h | 14 ++- src/libvirt_private.syms | 2 + src/storage/storage_backend.h | 3 +- src/storage/storage_backend_fs.c | 3 +- src/storage/storage_backend_iscsi.c | 3 +- src/storage/storage_backend_logical.c | 3 +- src/storage/storage_backend_mpath.c | 3 +- src/storage/storage_backend_scsi.c | 3 +- src/storage/storage_backend_zfs.c | 3 +- src/storage/storage_driver.c | 166 +++++++++++++++++++----- 11 files changed, 348 insertions(+), 84 deletions(-) -- 1.9.3

3 20

[libvirt] [PATCH] virsh: remove stale comment
by Ján Tomko 02 Apr '15

02 Apr '15

Copied from the vcpupin command, which has two modes of operation. --- Pushed as trivial. tools/virsh-domain.c | 1 - 1 file changed, 1 deletion(-) diff --git a/tools/virsh-domain.c b/tools/virsh-domain.c index eee441f..e7b5eeb 100644 --- a/tools/virsh-domain.c +++ b/tools/virsh-domain.c @@ -6961,7 +6961,6 @@ cmdIOThreadPin(vshControl *ctl, const vshCmd *cmd) goto cleanup; cpumaplen = VIR_CPU_MAPLEN(maxcpu); - /* Pin mode: pinning specified vcpu to specified physical cpus*/ if (!(cpumap = vshParseCPUList(ctl, cpulist, maxcpu, cpumaplen))) goto cleanup; -- 2.0.5

1 0

[libvirt] [python PATCH] Post-release version bump to 1.2.15
by Jiri Denemark 02 Apr '15

02 Apr '15

Signed-off-by: Jiri Denemark <jdenemar(a)redhat.com> --- Pushed as trivial. setup.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/setup.py b/setup.py index 00e6d75..79b048f 100755 --- a/setup.py +++ b/setup.py @@ -309,7 +309,7 @@ class my_clean(clean): _c_modules, _py_modules = get_module_lists() setup(name = 'libvirt-python', - version = '1.2.14', + version = '1.2.15', url = 'http://www.libvirt.org', maintainer = 'Libvirt Maintainers', maintainer_email = 'libvir-list(a)redhat.com', -- 2.3.5

1 0