- Devel - libvirt List Archives

Re: [libvirt] Building libvirt under Windows
by Matthias Bolte 16 Apr '15

16 Apr '15

2015-04-16 11:52 GMT+02:00 Pavel Fedin <p.fedin(a)samsung.com>: > Hello! > > I am currently building libvirt under Windows64, and i would like to ask > for correct way to do it. > The first thing to resolve is absence of RPC XDR library. From configure > source i figured out that portablexdr library may help. I have downloaded > it, but it comes with a different RPC code generator named portable-rpcgen. > I managed to use it, but i had to patch both libvirt (in order to recognize > this generator and handle it properly) and portable-rpcgen (in order to > correctly work under Windows, as well as get rid of some crashes). > Initially i wanted to upstream portablexdr fixes and emailed the author. > But the author replied that he is not going to fix portablexdr because it's > now deprecated, since original XDR code comes under free license now. So, i > have all necessary fixes in my own fork. He suggested me to come here to ask > about these things. > So, guys, how do you do it ? I cannot find any rpcgen package usable under > Windows (MinGW, not Cygwin). Hi, In 2010 I wrote a set of scripts to build libvirt on Windows using MinGW. Sou can find them here: https://github.com/photron/msys_setup I haven't update them in the last two years, so they might not work out-of-the-box anymore. But it might still be helpful to look at them and see how I did things. Regarding XDR: I used portablexdr. It worked for me, I can't remember if I had to patch it or not. -- Matthias Bolte http://photron.blogspot.com

1 0

[libvirt] util: client: shall libvirtd record task histories?
by zhang bo 16 Apr '15

16 Apr '15

We recently encountered a problem: 1) migrate a domain 2) the client unexpectedly got *crashed* (let's take it as virsh command) 3) *libvirtd still kept migrating the domain* 4) after it's restarted, the client didn't know the guest is still migrating. The problem is that libvirtd and the client has different view of the task state. After migration, the client may wrongly think that something's wrong that the domain got unexpectedly migrated. In my opinion, libvirtd should just *execute* tasks, like the hands of a human, while clients should be the brain to *schedule and remember* tasks. So, In order to avoid this problem,we should let the client record all the taskes somewhere, and reload the states after its restart. the client may cancel or continue the task as it wishes. Libvirtd should not record the task status. What's your opinions? thanks in advance.

3 3

[libvirt] [PATCH] qemu: bulk stats: Ignore errors from missing/inaccessible disks
by Peter Krempa 16 Apr '15

16 Apr '15

Rather than erroring out make the best attempt to retrieve other data if disks are inaccessible or missing. The failure will still be logged though. Since the bulk stats API is called on multiple domains an error like this makes the API unusable. This regression was introduced by commit 596a13713420e01b20ce3dc3fdbe06d073682675 Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1209394 --- src/qemu/qemu_driver.c | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index f5a3ef9..e79e2a9 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -19275,8 +19275,13 @@ qemuDomainGetStatsOneBlock(virQEMUDriverPtr driver, ret = 0; goto cleanup; } - if (qemuStorageLimitsRefresh(driver, cfg, dom, src) < 0) + + if (qemuStorageLimitsRefresh(driver, cfg, dom, src) < 0) { + virResetLastError(); + ret = 0; goto cleanup; + } + if (src->allocation) QEMU_ADD_BLOCK_PARAM_ULL(record, maxparams, block_idx, "allocation", src->allocation); -- 2.3.5

2 2

[libvirt] [java PATCH] added pom details
by Laszlo Hornyak 15 Apr '15

15 Apr '15

The added details are required in order to upload to maven central Signed-off-by: Laszlo Hornyak <laszlo.hornyak(a)gmail.com> --- pom.xml.in | 14 ++++++++++++++ 1 file changed, 14 insertions(+) diff --git a/pom.xml.in b/pom.xml.in index 4f49a3a..2ac8822 100644 --- a/pom.xml.in +++ b/pom.xml.in @@ -12,6 +12,18 @@ <name>libvirt java bindings</name> <description>Java API for the libvirt C library</description> <url>http://www.libvirt.org</url> + <developers> + <developer> + <id>veillard(a)redhat.com</id> + <email>veillard(a)redhat.com</email> + <organization>redhat</organization> + <organizationUrl>http://www.redhat.com</organizationUrl> + </developer> + </developers> + <organization> + <name>Libvirt</name> + <url>https://libvirt.org/</url> + </organization> <licenses> <license> <name>MIT license</name> @@ -20,6 +32,8 @@ </licenses> <scm> <url>http://www.libvirt.org/git/?p=libvirt-java.git;a=summary</url> + <developerConnection>scm:git://libvirt.org/libvirt-java.git</developerConnection> + <connection>scm:git://libvirt.org/libvirt-java.git</connection> </scm> <dependencies> -- 1.9.3

1 0

[libvirt] [PATCH] added pom details
by Laszlo Hornyak 15 Apr '15

15 Apr '15

The added details are required in order to upload to maven central Signed-off-by: Laszlo Hornyak <laszlo.hornyak(a)gmail.com> --- pom.xml.in | 14 ++++++++++++++ 1 file changed, 14 insertions(+) diff --git a/pom.xml.in b/pom.xml.in index 4f49a3a..2ac8822 100644 --- a/pom.xml.in +++ b/pom.xml.in @@ -12,6 +12,18 @@ <name>libvirt java bindings</name> <description>Java API for the libvirt C library</description> <url>http://www.libvirt.org</url> + <developers> + <developer> + <id>veillard(a)redhat.com</id> + <email>veillard(a)redhat.com</email> + <organization>redhat</organization> + <organizationUrl>http://www.redhat.com</organizationUrl> + </developer> + </developers> + <organization> + <name>Libvirt</name> + <url>https://libvirt.org/</url> + </organization> <licenses> <license> <name>MIT license</name> @@ -20,6 +32,8 @@ </licenses> <scm> <url>http://www.libvirt.org/git/?p=libvirt-java.git;a=summary</url> + <developerConnection>scm:git://libvirt.org/libvirt-java.git</developerConnection> + <connection>scm:git://libvirt.org/libvirt-java.git</connection> </scm> <dependencies> -- 1.9.3

2 2

[libvirt] [PATCH] Fix virCgroupGetPercpuStats with non-continuous present CPUs
by Ján Tomko 15 Apr '15

15 Apr '15

Per-cpu stats are only shown for present CPUs in the cgroups, but we were only parsing the largest CPU number from /sys/devices/system/cpu/present and looking for stats even for non-present CPUs. This resulted in: internal error: cpuacct parse error --- cfg.mk | 2 +- src/nodeinfo.c | 17 +++++++++++++++++ src/nodeinfo.h | 1 + src/util/vircgroup.c | 24 ++++++++++++++++++------ tests/vircgroupmock.c | 44 ++++++++++++++++++++++++++++++++++++++------ tests/vircgrouptest.c | 47 +++++++++++++++++++++++++++++++++++------------ 6 files changed, 110 insertions(+), 25 deletions(-) diff --git a/cfg.mk b/cfg.mk index 21f83c3..70612f8 100644 --- a/cfg.mk +++ b/cfg.mk @@ -1095,7 +1095,7 @@ exclude_file_name_regexp--sc_prohibit_asprintf = \ ^(bootstrap.conf$$|src/util/virstring\.[ch]$$|tests/vircgroupmock\.c$$) exclude_file_name_regexp--sc_prohibit_strdup = \ - ^(docs/|examples/|src/util/virstring\.c|tests/virnetserverclientmock.c$$) + ^(docs/|examples/|src/util/virstring\.c|tests/vir(netserverclient|cgroup)mock.c$$) exclude_file_name_regexp--sc_prohibit_close = \ (\.p[yl]$$|\.spec\.in$$|^docs/|^(src/util/virfile\.c|src/libvirt-stream\.c|tests/vir(cgroup|pci)mock\.c)$$) diff --git a/src/nodeinfo.c b/src/nodeinfo.c index 3c22ebc..3a27c22 100644 --- a/src/nodeinfo.c +++ b/src/nodeinfo.c @@ -1242,6 +1242,23 @@ nodeGetCPUCount(void) } virBitmapPtr +nodeGetPresentCPUBitmap(void) +{ + int max_present; + + if ((max_present = nodeGetCPUCount()) < 0) + return NULL; + +#ifdef __linux__ + if (virFileExists(SYSFS_SYSTEM_PATH "/cpu/present")) + return linuxParseCPUmap(max_present, SYSFS_SYSTEM_PATH "/cpu/present"); +#endif + virReportError(VIR_ERR_NO_SUPPORT, "%s", + _("non-continuous host cpu numbers not implemented on this platform")); + return NULL; +} + +virBitmapPtr nodeGetCPUBitmap(int *max_id ATTRIBUTE_UNUSED) { #ifdef __linux__ diff --git a/src/nodeinfo.h b/src/nodeinfo.h index a993c63..047bd5c 100644 --- a/src/nodeinfo.h +++ b/src/nodeinfo.h @@ -43,6 +43,7 @@ int nodeGetCellsFreeMemory(unsigned long long *freeMems, int nodeGetMemory(unsigned long long *mem, unsigned long long *freeMem); +virBitmapPtr nodeGetPresentCPUBitmap(void); virBitmapPtr nodeGetCPUBitmap(int *max_id); int nodeGetCPUCount(void); diff --git a/src/util/vircgroup.c b/src/util/vircgroup.c index fe34290..e65617a 100644 --- a/src/util/vircgroup.c +++ b/src/util/vircgroup.c @@ -2985,7 +2985,8 @@ static int virCgroupGetPercpuVcpuSum(virCgroupPtr group, unsigned int nvcpupids, unsigned long long *sum_cpu_time, - unsigned int num) + size_t nsum, + virBitmapPtr cpumap) { int ret = -1; size_t i; @@ -2995,7 +2996,7 @@ virCgroupGetPercpuVcpuSum(virCgroupPtr group, for (i = 0; i < nvcpupids; i++) { char *pos; unsigned long long tmp; - size_t j; + ssize_t j; if (virCgroupNewVcpu(group, i, false, &group_vcpu) < 0) goto cleanup; @@ -3004,7 +3005,9 @@ virCgroupGetPercpuVcpuSum(virCgroupPtr group, goto cleanup; pos = buf; - for (j = 0; j < num; j++) { + for (j = virBitmapNextSetBit(cpumap, -1); + j >= 0 && j < nsum; + j = virBitmapNextSetBit(cpumap, j)) { if (virStrToLong_ull(pos, &pos, 10, &tmp) < 0) { virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("cpuacct parse error")); @@ -3042,6 +3045,7 @@ virCgroupGetPercpuStats(virCgroupPtr group, virTypedParameterPtr ent; int param_idx; unsigned long long cpu_time; + virBitmapPtr cpumap = NULL; /* return the number of supported params */ if (nparams == 0 && ncpus != 0) { @@ -3052,9 +3056,11 @@ virCgroupGetPercpuStats(virCgroupPtr group, } /* To parse account file, we need to know how many cpus are present. */ - if ((total_cpus = nodeGetCPUCount()) < 0) + if (!(cpumap = nodeGetPresentCPUBitmap())) return rv; + total_cpus = virBitmapSize(cpumap); + if (ncpus == 0) return total_cpus; @@ -3077,7 +3083,11 @@ virCgroupGetPercpuStats(virCgroupPtr group, need_cpus = MIN(total_cpus, start_cpu + ncpus); for (i = 0; i < need_cpus; i++) { - if (virStrToLong_ull(pos, &pos, 10, &cpu_time) < 0) { + bool present; + ignore_value(virBitmapGetBit(cpumap, i, &present)); + if (!present) { + cpu_time = 0; + } else if (virStrToLong_ull(pos, &pos, 10, &cpu_time) < 0) { virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("cpuacct parse error")); goto cleanup; @@ -3097,7 +3107,8 @@ virCgroupGetPercpuStats(virCgroupPtr group, if (VIR_ALLOC_N(sum_cpu_time, need_cpus) < 0) goto cleanup; - if (virCgroupGetPercpuVcpuSum(group, nvcpupids, sum_cpu_time, need_cpus) < 0) + if (virCgroupGetPercpuVcpuSum(group, nvcpupids, sum_cpu_time, need_cpus, + cpumap) < 0) goto cleanup; for (i = start_cpu; i < need_cpus; i++) { @@ -3113,6 +3124,7 @@ virCgroupGetPercpuStats(virCgroupPtr group, rv = param_idx + 1; cleanup: + virBitmapFree(cpumap); VIR_FREE(sum_cpu_time); VIR_FREE(buf); return rv; diff --git a/tests/vircgroupmock.c b/tests/vircgroupmock.c index 6e0a0e7..f2fee7d 100644 --- a/tests/vircgroupmock.c +++ b/tests/vircgroupmock.c @@ -51,6 +51,8 @@ const char *fakedevicedir1 = FAKEDEVDIR1; # define SYSFS_PREFIX "/not/really/sys/fs/cgroup/" +# define SYSFS_CPU_PRESENT "/sys/devices/system/cpu/present" +# define SYSFS_CPU_PRESENT_MOCKED "devices_system_cpu_present" /* * The plan: @@ -215,7 +217,15 @@ static int make_controller(const char *path, mode_t mode) "user 216687025\n" "system 43421396\n"); MAKE_FILE("cpuacct.usage", "2787788855799582\n"); - MAKE_FILE("cpuacct.usage_percpu", "1413142688153030 1374646168910542\n"); + MAKE_FILE("cpuacct.usage_percpu", + "7059492996 0 0 0 0 0 0 0 4180532496 0 0 0 0 0 0 0 " + "1957541268 0 0 0 0 0 0 0 2065932204 0 0 0 0 0 0 0 " + "18228689414 0 0 0 0 0 0 0 4245525148 0 0 0 0 0 0 0 " + "2911161568 0 0 0 0 0 0 0 1407758136 0 0 0 0 0 0 0 " + "1836807700 0 0 0 0 0 0 0 1065296618 0 0 0 0 0 0 0 " + "2046213266 0 0 0 0 0 0 0 747889778 0 0 0 0 0 0 0 " + "709566900 0 0 0 0 0 0 0 444777342 0 0 0 0 0 0 0 " + "5683512916 0 0 0 0 0 0 0 635751356 0 0 0 0 0 0 0\n"); } else if (STRPREFIX(controller, "cpuset")) { MAKE_FILE("cpuset.cpu_exclusive", "1\n"); if (STREQ(controller, "cpuset")) @@ -431,6 +441,9 @@ static void init_sysfs(void) MAKE_CONTROLLER("blkio"); MAKE_CONTROLLER("memory"); MAKE_CONTROLLER("freezer"); + + if (make_file(fakesysfsdir, SYSFS_CPU_PRESENT_MOCKED, "8-23,48-159\n") < 0) + abort(); } @@ -623,21 +636,27 @@ int __xstat(int ver, const char *path, struct stat *sb) int stat(const char *path, struct stat *sb) { + char *newpath = NULL; int ret; init_syms(); - if (STRPREFIX(path, SYSFS_PREFIX)) { + if (STREQ(path, SYSFS_CPU_PRESENT)) { + init_sysfs(); + if (asprintf(&newpath, "%s/%s", + fakesysfsdir, + SYSFS_CPU_PRESENT_MOCKED) < 0) { + errno = ENOMEM; + return -1; + } + } else if (STRPREFIX(path, SYSFS_PREFIX)) { init_sysfs(); - char *newpath; if (asprintf(&newpath, "%s/%s", fakesysfsdir, path + strlen(SYSFS_PREFIX)) < 0) { errno = ENOMEM; return -1; } - ret = realstat(newpath, sb); - free(newpath); } else if (STRPREFIX(path, fakedevicedir0)) { sb->st_mode = S_IFBLK; sb->st_rdev = makedev(8, 0); @@ -647,8 +666,11 @@ int stat(const char *path, struct stat *sb) sb->st_rdev = makedev(9, 0); return 0; } else { - ret = realstat(path, sb); + if (!(newpath = strdup(path))) + return -1; } + ret = realstat(newpath, sb); + free(newpath); return ret; } @@ -682,6 +704,16 @@ int open(const char *path, int flags, ...) init_syms(); + if (STREQ(path, SYSFS_CPU_PRESENT)) { + init_sysfs(); + if (asprintf(&newpath, "%s/%s", + fakesysfsdir, + SYSFS_CPU_PRESENT_MOCKED) < 0) { + errno = ENOMEM; + return -1; + } + } + if (STRPREFIX(path, SYSFS_PREFIX)) { init_sysfs(); if (asprintf(&newpath, "%s/%s", diff --git a/tests/vircgrouptest.c b/tests/vircgrouptest.c index 35ac0c0..b65ea3f 100644 --- a/tests/vircgrouptest.c +++ b/tests/vircgrouptest.c @@ -538,12 +538,35 @@ static int testCgroupGetPercpuStats(const void *args ATTRIBUTE_UNUSED) virCgroupPtr cgroup = NULL; size_t i; int rv, ret = -1; - virTypedParameter params[2]; + virTypedParameterPtr params = NULL; +# define EXPECTED_NCPUS 160 - // TODO: mock nodeGetCPUCount() as well & check 2nd cpu, too unsigned long long expected[] = { - 1413142688153030ULL + 0, 0, 0, 0, 0, 0, 0, 0, + 7059492996, 0, 0, 0, 0, 0, 0, 0, + 4180532496, 0, 0, 0, 0, 0, 0, 0, + 0, 0, 0, 0, 0, 0, 0, 0, + 0, 0, 0, 0, 0, 0, 0, 0, + 0, 0, 0, 0, 0, 0, 0, 0, + 1957541268, 0, 0, 0, 0, 0, 0, 0, + 2065932204, 0, 0, 0, 0, 0, 0, 0, + 18228689414, 0, 0, 0, 0, 0, 0, 0, + 4245525148, 0, 0, 0, 0, 0, 0, 0, + 2911161568, 0, 0, 0, 0, 0, 0, 0, + 1407758136, 0, 0, 0, 0, 0, 0, 0, + 1836807700, 0, 0, 0, 0, 0, 0, 0, + 1065296618, 0, 0, 0, 0, 0, 0, 0, + 2046213266, 0, 0, 0, 0, 0, 0, 0, + 747889778, 0, 0, 0, 0, 0, 0, 0, + 709566900, 0, 0, 0, 0, 0, 0, 0, + 444777342, 0, 0, 0, 0, 0, 0, 0, + 5683512916, 0, 0, 0, 0, 0, 0, 0, + 635751356, 0, 0, 0, 0, 0, 0, 0, }; + verify(ARRAY_CARDINALITY(expected) == EXPECTED_NCPUS); + + if (VIR_ALLOC_N(params, EXPECTED_NCPUS) < 0) + goto cleanup; if ((rv = virCgroupNewPartition("/virtualmachines", true, (1 << VIR_CGROUP_CONTROLLER_CPU) | @@ -553,37 +576,37 @@ static int testCgroupGetPercpuStats(const void *args ATTRIBUTE_UNUSED) goto cleanup; } - if (nodeGetCPUCount() < 1) { + if (nodeGetCPUCount() != EXPECTED_NCPUS) { fprintf(stderr, "Unexpected: nodeGetCPUCount() yields: %d\n", nodeGetCPUCount()); goto cleanup; } if ((rv = virCgroupGetPercpuStats(cgroup, params, - 2, 0, 1, 0)) < 0) { + 1, 0, EXPECTED_NCPUS, 0)) < 0) { fprintf(stderr, "Failed call to virCgroupGetPercpuStats for /virtualmachines cgroup: %d\n", -rv); goto cleanup; } - for (i = 0; i < ARRAY_CARDINALITY(expected); i++) { + for (i = 0; i < EXPECTED_NCPUS; i++) { if (!STREQ(params[i].field, VIR_DOMAIN_CPU_STATS_CPUTIME)) { fprintf(stderr, - "Wrong parameter name value from virCgroupGetPercpuStats (is: %s)\n", - params[i].field); + "Wrong parameter name value from virCgroupGetPercpuStats at %zu (is: %s)\n", + i, params[i].field); goto cleanup; } if (params[i].type != VIR_TYPED_PARAM_ULLONG) { fprintf(stderr, - "Wrong parameter value type from virCgroupGetPercpuStats (is: %d)\n", - params[i].type); + "Wrong parameter value type from virCgroupGetPercpuStats at %zu (is: %d)\n", + i, params[i].type); goto cleanup; } if (params[i].value.ul != expected[i]) { fprintf(stderr, - "Wrong value from virCgroupGetMemoryUsage (expected %llu)\n", - params[i].value.ul); + "Wrong value from virCgroupGetMemoryUsage at %zu (expected %llu)\n", + i, params[i].value.ul); goto cleanup; } } -- 2.0.4

3 4

[libvirt] [PATCH] tests: fix build on old 32-bit platforms
by Eric Blake 15 Apr '15

15 Apr '15

gcc 4.1.2 (hello RHEL 5) on 32-bit platforms complains: vircgrouptest.c: In function 'testCgroupGetPercpuStats': vircgrouptest.c:627: warning: integer constant is too large for 'long' type vircgrouptest.c:628: warning: this decimal constant is unsigned only in ISO C90 vircgrouptest.c:634: warning: integer constant is too large for 'long' type vircgrouptest.c:635: warning: this decimal constant is unsigned only in ISO C90 vircgrouptest.c:636: warning: this decimal constant is unsigned only in ISO C90 vircgrouptest.c:644: warning: integer constant is too large for 'long' type * tests/vircgrouptest.c (testCgroupGetPercpuStats): Use ULL suffix. Signed-off-by: Eric Blake <eblake(a)redhat.com> --- Pushing under the build-breaker rule tests/vircgrouptest.c | 34 +++++++++++++++++----------------- 1 file changed, 17 insertions(+), 17 deletions(-) diff --git a/tests/vircgrouptest.c b/tests/vircgrouptest.c index 2b30258..ad49425 100644 --- a/tests/vircgrouptest.c +++ b/tests/vircgrouptest.c @@ -1,5 +1,5 @@ /* - * Copyright (C) 2013-2014 Red Hat, Inc. + * Copyright (C) 2013-2015 Red Hat, Inc. * * This library is free software; you can redistribute it and/or * modify it under the terms of the GNU Lesser General Public @@ -624,25 +624,25 @@ static int testCgroupGetPercpuStats(const void *args ATTRIBUTE_UNUSED) unsigned long long expected[EXPECTED_NCPUS] = { 0, 0, 0, 0, 0, 0, 0, 0, - 7059492996, 0, 0, 0, 0, 0, 0, 0, - 4180532496, 0, 0, 0, 0, 0, 0, 0, + 7059492996ULL, 0, 0, 0, 0, 0, 0, 0, + 4180532496ULL, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, - 1957541268, 0, 0, 0, 0, 0, 0, 0, - 2065932204, 0, 0, 0, 0, 0, 0, 0, - 18228689414, 0, 0, 0, 0, 0, 0, 0, - 4245525148, 0, 0, 0, 0, 0, 0, 0, - 2911161568, 0, 0, 0, 0, 0, 0, 0, - 1407758136, 0, 0, 0, 0, 0, 0, 0, - 1836807700, 0, 0, 0, 0, 0, 0, 0, - 1065296618, 0, 0, 0, 0, 0, 0, 0, - 2046213266, 0, 0, 0, 0, 0, 0, 0, - 747889778, 0, 0, 0, 0, 0, 0, 0, - 709566900, 0, 0, 0, 0, 0, 0, 0, - 444777342, 0, 0, 0, 0, 0, 0, 0, - 5683512916, 0, 0, 0, 0, 0, 0, 0, - 635751356, 0, 0, 0, 0, 0, 0, 0, + 1957541268ULL, 0, 0, 0, 0, 0, 0, 0, + 2065932204ULL, 0, 0, 0, 0, 0, 0, 0, + 18228689414ULL, 0, 0, 0, 0, 0, 0, 0, + 4245525148ULL, 0, 0, 0, 0, 0, 0, 0, + 2911161568ULL, 0, 0, 0, 0, 0, 0, 0, + 1407758136ULL, 0, 0, 0, 0, 0, 0, 0, + 1836807700ULL, 0, 0, 0, 0, 0, 0, 0, + 1065296618ULL, 0, 0, 0, 0, 0, 0, 0, + 2046213266ULL, 0, 0, 0, 0, 0, 0, 0, + 747889778ULL, 0, 0, 0, 0, 0, 0, 0, + 709566900ULL, 0, 0, 0, 0, 0, 0, 0, + 444777342ULL, 0, 0, 0, 0, 0, 0, 0, + 5683512916ULL, 0, 0, 0, 0, 0, 0, 0, + 635751356ULL, 0, 0, 0, 0, 0, 0, 0, }; if (VIR_ALLOC_N(params, EXPECTED_NCPUS) < 0) -- 2.1.0

1 0

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

15 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> --- diff to v1: -hopefully this one is race free src/rpc/virnetsocket.c | 148 ++++++++++++------------------------------------- 1 file changed, 36 insertions(+), 112 deletions(-) diff --git a/src/rpc/virnetsocket.c b/src/rpc/virnetsocket.c index 6b019cc..2b891ae 100644 --- a/src/rpc/virnetsocket.c +++ b/src/rpc/virnetsocket.c @@ -123,7 +123,7 @@ VIR_ONCE_GLOBAL_INIT(virNetSocket) #ifndef WIN32 -static int virNetSocketForkDaemon(const char *binary, int passfd) +static int virNetSocketForkDaemon(const char *binary) { int ret; virCommandPtr cmd = virCommandNewArgList(binary, @@ -136,10 +136,6 @@ static int virNetSocketForkDaemon(const char *binary, int passfd) virCommandAddEnvPassBlockSUID(cmd, "XDG_RUNTIME_DIR", NULL); virCommandClearCaps(cmd); virCommandDaemonize(cmd); - if (passfd) { - virCommandPassFD(cmd, passfd, VIR_COMMAND_PASS_FD_CLOSE_PARENT); - virCommandPassListenFDs(cmd); - } ret = virCommandRun(cmd, NULL); virCommandFree(cmd); return ret; @@ -543,10 +539,10 @@ int virNetSocketNewConnectUNIX(const char *path, const char *binary, virNetSocketPtr *retsock) { - char *binname = NULL; - char *pidpath = NULL; - int fd, passfd = -1; - int pidfd = -1; + char *lockpath = NULL; + int lockfd = -1; + int fd = -1; + int retries = 100; virSocketAddr localAddr; virSocketAddr remoteAddr; @@ -561,6 +557,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; @@ -574,108 +586,25 @@ int virNetSocketNewConnectUNIX(const char *path, if (remoteAddr.data.un.sun_path[0] == '@') remoteAddr.data.un.sun_path[0] = '\0'; - retry: - if (connect(fd, &remoteAddr.data.sa, remoteAddr.len) < 0) { - int status = 0; - pid_t pid = 0; - - if (!spawnDaemon) { + while (retries && + connect(fd, &remoteAddr.data.sa, remoteAddr.len) < 0) { + if (!(spawnDaemon && errno == ENOENT)) { virReportSystemError(errno, _("Failed to connect socket to '%s'"), path); goto error; } - if (!(binname = last_component(binary)) || binname[0] == '\0') { - virReportError(VIR_ERR_INTERNAL_ERROR, - _("Cannot determine basename for binary '%s'"), - binary); - goto error; - } - - 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 ((passfd = socket(PF_UNIX, SOCK_STREAM, 0)) < 0) { - virReportSystemError(errno, "%s", _("Failed to create socket")); + if (virNetSocketForkDaemon(binary) < 0) goto error; - } - /* - * We already even acquired the pidfile, so no one else should be using - * @path right now. So we're OK to unlink it and paying attention to - * the return value makes no real sense here. Only if it's not an - * abstract socket, of course. - */ - if (path[0] != '@') - unlink(path); - - /* - * We have to fork() here, because umask() is set per-process, chmod() - * is racy and fchmod() has undefined behaviour on sockets according to - * POSIX, so it doesn't work outside Linux. - */ - if ((pid = virFork()) < 0) - goto error; - - if (pid == 0) { - umask(0077); - if (bind(passfd, &remoteAddr.data.sa, remoteAddr.len) < 0) - _exit(EXIT_FAILURE); - - _exit(EXIT_SUCCESS); - } + retries--; + usleep(5000); + } - if (virProcessWait(pid, &status, false) < 0) - goto error; - - if (status != EXIT_SUCCESS) { - /* - * OK, so the child failed to bind() the socket. This may mean that - * another daemon was starting at the same time and succeeded with - * its bind() (even though it should not happen because we using a - * pidfile for the race). So we'll try connecting again, but this - * time without spawning the daemon. - */ - spawnDaemon = false; - virPidFileDeletePath(pidpath); - VIR_FORCE_CLOSE(pidfd); - VIR_FORCE_CLOSE(passfd); - goto retry; - } - - if (listen(passfd, 0) < 0) { - virReportSystemError(errno, "%s", - _("Failed to listen on socket that's about " - "to be passed to the daemon")); - goto error; - } - - if (connect(fd, &remoteAddr.data.sa, remoteAddr.len) < 0) { - virReportSystemError(errno, _("Failed to connect socket to '%s'"), - 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); @@ -687,19 +616,14 @@ int virNetSocketNewConnectUNIX(const char *path, if (!(*retsock = virNetSocketNew(&localAddr, &remoteAddr, true, fd, -1, 0))) goto error; - VIR_FREE(pidpath); - return 0; error: - if (pidfd >= 0) - virPidFileDeletePath(pidpath); - VIR_FREE(pidpath); + if (lockfd) + unlink(lockpath); + VIR_FREE(lockpath); VIR_FORCE_CLOSE(fd); - VIR_FORCE_CLOSE(passfd); - VIR_FORCE_CLOSE(pidfd); - if (spawnDaemon) - unlink(path); + VIR_FORCE_CLOSE(lockfd); return -1; } #else -- 2.0.5

4 3

[libvirt] [PATCHv2 0/5] Strip control codes from virBufferEscapeString
by Ján Tomko 15 Apr '15

15 Apr '15

For https://bugzilla.redhat.com/show_bug.cgi?id=1184131 https://bugzilla.redhat.com/show_bug.cgi?id=1066564 v1: https://www.redhat.com/archives/libvir-list/2015-March/msg01529.html v2: * added patches 1-4 that should prevent us from calling virBufferEscapeString with control codes * patch 5 is unchanged Ján Tomko (5): tests: rename testStripIPv6BracketsData to testStripData Add functions dealing with control characters in strings Strip control characters from sysfs attributes Ignore storage volumes with control codes in their names Strip control codes in virBufferEscapeString src/libvirt_private.syms | 2 ++ src/node_device/node_device_udev.c | 2 ++ src/storage/storage_backend_fs.c | 6 +++++ src/util/virbuffer.c | 14 ++++++++--- src/util/virstring.c | 39 ++++++++++++++++++++++++++++++ src/util/virstring.h | 2 ++ tests/virbuftest.c | 49 ++++++++++++++++++++++++++++++++++++++ tests/virstringtest.c | 45 +++++++++++++++++++++++++++++++--- 8 files changed, 153 insertions(+), 6 deletions(-) -- 2.0.5

2 11

[libvirt] [PATCH 0/2] Couple of virnetdevtest cleanups
by Michal Privoznik 15 Apr '15

15 Apr '15

*** BLURB HERE *** Michal Privoznik (2): tests: Add virnetdevtestdata to EXTRA_DIST Cleanup "/sys/class/net" usage src/Makefile.am | 4 ++-- src/parallels/parallels_network.c | 3 +-- src/util/virnetdev.c | 5 ++--- src/util/virnetdev.h | 2 ++ src/util/virnetdevbridge.c | 1 - src/util/virnetdevmacvlan.c | 28 ++++++++++++++-------------- tests/Makefile.am | 1 + 7 files changed, 22 insertions(+), 22 deletions(-) -- 2.0.5

2 5