June 2016 - Devel - libvirt List Archives

[libvirt] Switch to a time based version number rule
by Daniel P. Berrange 14 Jun '16

14 Jun '16

Currently libvirt uses a 3 digit version number, except when it uses a 4 digit version number. We have the following rules - major - no one has any clue about when we should bump this - minor - bump this when some "significant"[*] features appear - micro - bump this on each new release - extra - bump this for stable branch releases [*] for a definition of "significant" that no one knows Now consider our actual requirements - A number that increments on each monthly release - A number that can be incremented for stable branch releases Ok, the micro + extra digits deal with our two actual requirements, so one may ask what is the point of the major + minor digits ? In 11 years of libvirt development we've only bumped the major digit once, and we didn't have any real reason why we chose to the bump the major digit, instead of continuing to bump the minor digit. It just felt like we ought to have a 1.0 release after 7+ years. Our decisions about when to bump the minor digit have not been that much less arbitray. We just look at what features are around and randomly decide if any feel "big enough" to justify a minor digit bump. Way back in the early days of libvirt, we had exactly this kind of mess when deciding /when/ to actually make releases. Sometimes we'd release after a month, sometimes after 3 months, completely arbitrarily based on whether the chances felt "big enough" to justify a release. Feature based release schedules are insanity and so we wised up and adopted the time base release schedule where we release monthly (except over xmas/new year period). I venture to suggest that the reasons for switching from feature to time based release schedules, also apply to version numbers. IOW we should switch to a time based version number change rule, instead of a feature based version number change rule. So what I'm suggesting is that we adopt the following rule - major: bumped for the first release of each year - minor: bumped for every major release - micro: bumped for stable branch releases Rather than wait until next January to adopt this rule, I'd suggest we pretend that it is January now, and thus switch our next version number to be 2.0.0, and jump to 3.0.0 in January 2017. IOW, over the next 2 years we'll do the following releases off master: - Jul 1, 2016 - 2.0.0 - Aug 1, 2016 - 2.1.0 - Sep 1, 2016 - 2.2.0 - Oct 1, 2016 - 2.3.0 - Nov 1, 2016 - 2.4.0 - Dec 1, 2016 - 2.5.0 - Jan 15, 2017 - 3.0.0 - Mar 1, 2017 - 3.1.0 - Apr 1, 2017 - 3.2.0 - May 1, 2017 - 3.3.0 - Jun 1, 2017 - 3.4.0 - Jul 1, 2017 - 3.5.0 - Aug 1, 2017 - 3.6.0 - Sep 1, 2017 - 3.7.0 - Oct 1, 2017 - 3.8.0 - Nov 1, 2017 - 3.9.0 - Dec 1, 2017 - 3.10.0 - Jan 15, 2018 - 4.0.0 - Mar 1, 2018 - 4.1.0 - Apr 1, 2018 - 4.2.0 - .... The stable release branch naming would just use the first major + minor digit in its name.. eg the stable branch for 2.3.0 would use v2.3-maint and do stable releases 2.3.1, 2.3.2, 2.3.3, etc. Regards, Daniel -- |: http://berrange.com -o- http://www.flickr.com/photos/dberrange/ :| |: http://libvirt.org -o- http://virt-manager.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|

8 16

[libvirt] [PATCH] qemu: Allow ACPI shutdown only for running domains
by Martin Kletzander 14 Jun '16

14 Jun '16

If the domain is not running, but for example the CPUs are stopped, the ACPI event gets queued and resume of the domain will just shut it off. https://bugzilla.redhat.com/show_bug.cgi?id=1216281 Signed-off-by: Martin Kletzander <mkletzan(a)redhat.com> --- src/qemu/qemu_driver.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index efb3f854fb10..eb38843c62d8 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -2042,7 +2042,7 @@ static int qemuDomainShutdownFlags(virDomainPtr dom, unsigned int flags) if (qemuDomainObjBeginJob(driver, vm, QEMU_JOB_MODIFY) < 0) goto cleanup; - if (!virDomainObjIsActive(vm)) { + if (virDomainObjGetState(vm, NULL) != VIR_DOMAIN_RUNNING) { virReportError(VIR_ERR_OPERATION_INVALID, "%s", _("domain is not running")); goto endjob; -- 2.8.4

2 1

[libvirt] [PATCH] Bump release to 2.0.0 and document release schedule & versioning
by Daniel P. Berrange 14 Jun '16

14 Jun '16

This bumps the release number of 2.0.0, to reflect the switch to a new time based release versioning scheme. The downloads page is updated to describe our policies for release schedules and release version numbering The stable release docs are changed to reflect the fact that the stable version numbers are now just 3 digits long instead of 4. Signed-off-by: Daniel P. Berrange <berrange(a)redhat.com> --- configure.ac | 2 +- docs/downloads.html.in | 59 ++++++++++++++++++++++++++++++++++++++++++++++---- src/libvirt_lxc.syms | 2 +- 3 files changed, 57 insertions(+), 6 deletions(-) diff --git a/configure.ac b/configure.ac index 827d9db..0114149 100644 --- a/configure.ac +++ b/configure.ac @@ -16,7 +16,7 @@ dnl You should have received a copy of the GNU Lesser General Public dnl License along with this library. If not, see dnl <http://www.gnu.org/licenses/>. -AC_INIT([libvirt], [1.3.6], [libvir-list(a)redhat.com] [], [http://libvirt.org] +AC_INIT([libvirt], [2.0.0], [libvir-list(a)redhat.com] [], [http://libvirt.org] AC_CONFIG_SRCDIR([src/libvirt.c]) AC_CONFIG_AUX_DIR([build-aux]) AC_CONFIG_HEADERS([config.h]) diff --git a/docs/downloads.html.in b/docs/downloads.html.in index 13a6db1..32cc2ec 100644 --- a/docs/downloads.html.in +++ b/docs/downloads.html.in @@ -32,20 +32,71 @@ <li><a href="http://libvirt.org/sources/libvirt-git-snapshot.tar.gz">libvirt.org HTTP server</a></li> </ul> + <h2><a name="schedule">Primary release schedule</a></h2> + + + Libvirt follows a time based plan, with releases made once a month + on the 1st of each month give or take a few days. The only exception + is at the start of the year where there are two 6 weeks gaps, giving + a total of 11 releases a year. Expect to see releases on approx: + + + <ul> + <li>Jan 15th</li> + <li>Mar 1st</li> + <li>Apr 1st</li> + <li>May 1st</li> + <li>Jun 1st</li> + <li>Jul 1st</li> + <li>Aug 1st</li> + <li>Sep 1st</li> + <li>Oct 1st</li> + <li>Nov 1st</li> + <li>Dec 1st</li> + </ul> + + <h2><a name="numbering">Release numbering</a></h2> + + + Since libvirt 2.0.0, a time based version numbering rule + is applied. As such, the changes in version number have + do not have any implications wrt the scope of features + or bugfixes included, the stability of the code, or the + API / ABI compatibility (libvirt API / ABI is guaranteed + stable forever). The rules applied for changing the libvirt + version number are: + + + <ul> + <li><code>major</code> - incremented by 1 for the first release of the year (the Jan 15th release)</li> + <li><code>minor</code> - incremented by 1 for each monthly release from git master</li> + <li><code>micro</code> - always 0 for releases from git master, incremented by 1 for each stable maintenance release</li> + </ul> + + + Prior to to 2.0.0 the major/minor numbers were incremented + fairly arbitrarily, and maintenance releases appended a + fourth digit. + + <h2><a name="maintenance">Maintenance releases</a></h2> In the git repository are several stable maintenance branches, matching the - pattern <code>vmajor.minor.micro-maint</code>; + pattern <code>vmajor.minor-maint</code>; these branches are forked off the corresponding - <code>vmajor.minor.micro</code> formal + <code>vmajor.minor.0</code> formal release, and may have further releases of the - form <code>vmajor.minor.micro.rel</code>. + form <code>vmajor.minor.micro</code>. These maintenance branches should only contain bug fixes, and no new features, backported from the master branch, and are supported as long as at least one downstream distribution expresses interest in a given branch. These maintenance - branches are considered during CVE analysis. + branches are considered during CVE analysis. In contrast + to the primary releases which are made once a month, there + is no formal schedule for the maintenance releases, which + are made whenever there is a need to make available key + bugfixes to downstream consumers. diff --git a/src/libvirt_lxc.syms b/src/libvirt_lxc.syms index 56c24c0..9b418ee 100644 --- a/src/libvirt_lxc.syms +++ b/src/libvirt_lxc.syms @@ -21,7 +21,7 @@ LIBVIRT_LXC_1.0.4 { virDomainLxcEnterSecurityLabel; } LIBVIRT_LXC_1.0.2; -LIBVIRT_LXC_1.3.6 { +LIBVIRT_LXC_2.0.0 { global: virDomainLxcEnterCGroup; } LIBVIRT_LXC_1.0.4; -- 2.5.5

5 6

[libvirt] [PATCH] cputest: Get rid of the array of test functions
by Jiri Denemark 14 Jun '16

14 Jun '16

Signed-off-by: Jiri Denemark <jdenemar(a)redhat.com> --- tests/cputest.c | 134 +++++++++++++++++++++----------------------------------- 1 file changed, 51 insertions(+), 83 deletions(-) diff --git a/tests/cputest.c b/tests/cputest.c index f68a3ec..0175d01 100644 --- a/tests/cputest.c +++ b/tests/cputest.c @@ -54,31 +54,9 @@ enum cpuTestBoolWithError { YES = 1 }; -enum api { - API_COMPARE, - API_GUEST_DATA, - API_BASELINE, - API_UPDATE, - API_HAS_FEATURE, - API_HOST_CPUID, - API_GUEST_CPUID, - API_JSON_CPUID, -}; - -static const char *apis[] = { - "compare", - "guest data", - "baseline", - "update", - "has feature", - "host CPUID", - "guest CPUID", - "json CPUID", -}; struct data { const char *arch; - enum api api; const char *host; const char *name; const char **models; @@ -475,7 +453,7 @@ cpuTestHasFeature(const void *arg) static int -cpuTestCPUID(const void *arg) +cpuTestCPUID(bool guest, const void *arg) { const struct data *data = arg; int ret = -1; @@ -497,7 +475,7 @@ cpuTestCPUID(const void *arg) goto cleanup; cpu->arch = hostData->arch; - if (data->api == API_GUEST_CPUID) { + if (guest) { cpu->type = VIR_CPU_TYPE_GUEST; cpu->match = VIR_CPU_MATCH_EXACT; cpu->fallback = VIR_CPU_FALLBACK_FORBID; @@ -510,7 +488,7 @@ cpuTestCPUID(const void *arg) if (virAsprintf(&result, "cpuid-%s-%s", data->host, - data->api == API_HOST_CPUID ? "host" : "guest") < 0) + guest ? "guest" : "host") < 0) goto cleanup; ret = cpuTestCompareXML(data->arch, cpu, result, false); @@ -525,6 +503,20 @@ cpuTestCPUID(const void *arg) } +static int +cpuTestHostCPUID(const void *arg) +{ + return cpuTestCPUID(false, arg); +} + + +static int +cpuTestGuestCPUID(const void *arg) +{ + return cpuTestCPUID(true, arg); +} + + #if WITH_QEMU && WITH_YAJL static int cpuTestJSONCPUID(const void *arg) @@ -573,52 +565,6 @@ cpuTestJSONCPUID(const void *arg) #endif -static int (*cpuTest[])(const void *) = { - cpuTestCompare, - cpuTestGuestData, - cpuTestBaseline, - cpuTestUpdate, - cpuTestHasFeature, - cpuTestCPUID, - cpuTestCPUID, -#if WITH_QEMU && WITH_YAJL - cpuTestJSONCPUID, -#else - NULL, -#endif -}; - - -static int -cpuTestRun(const char *name, const struct data *data) -{ - char *label = NULL; - char *tmp; - - if (virAsprintf(&label, "CPU %s(%s): %s", apis[data->api], data->arch, name) < 0) - return -1; - - tmp = virTestLogContentAndReset(); - VIR_FREE(tmp); - - if (virTestRun(label, cpuTest[data->api], data) < 0) { - if (virTestGetDebug()) { - char *log; - if ((log = virTestLogContentAndReset()) && - strlen(log) > 0) - VIR_TEST_DEBUG("\n%s\n", log); - VIR_FREE(log); - } - - VIR_FREE(label); - return -1; - } - - VIR_FREE(label); - return 0; -} - - static const char *model486[] = { "486" }; static const char *nomodel[] = { "nomodel" }; static const char *models[] = { "qemu64", "core2duo", "Nehalem" }; @@ -640,23 +586,45 @@ mymain(void) #define DO_TEST(arch, api, name, host, cpu, \ models, nmodels, preferred, flags, result) \ do { \ - static struct data data = { \ - arch, api, host, cpu, models, \ + struct data data = { \ + arch, host, cpu, models, \ models == NULL ? NULL : #models, \ nmodels, preferred, flags, result \ }; \ - if (cpuTestRun(name, &data) < 0) \ + char *testLabel; \ + char *tmp; \ + \ + tmp = virTestLogContentAndReset(); \ + VIR_FREE(tmp); \ + \ + if (virAsprintf(&testLabel, "%s(%s): %s", \ + #api, arch, name) < 0) { \ ret = -1; \ + break; \ + } \ + \ + if (virTestRun(testLabel, api, &data) < 0) { \ + if (virTestGetDebug()) { \ + char *log; \ + if ((log = virTestLogContentAndReset()) && \ + strlen(log) > 0) \ + VIR_TEST_DEBUG("\n%s\n", log); \ + VIR_FREE(log); \ + } \ + ret = -1; \ + } \ + \ + VIR_FREE(testLabel); \ } while (0) #define DO_TEST_COMPARE(arch, host, cpu, result) \ - DO_TEST(arch, API_COMPARE, \ + DO_TEST(arch, cpuTestCompare, \ host "/" cpu " (" #result ")", \ host, cpu, NULL, 0, NULL, 0, result) #define DO_TEST_UPDATE(arch, host, cpu, result) \ do { \ - DO_TEST(arch, API_UPDATE, \ + DO_TEST(arch, cpuTestUpdate, \ cpu " on " host, \ host, cpu, NULL, 0, NULL, 0, 0); \ DO_TEST_COMPARE(arch, host, host "+" cpu, result); \ @@ -673,19 +641,19 @@ mymain(void) if (virAsprintf(&label, "%s%s", name, suffix) < 0) { \ ret = -1; \ } else { \ - DO_TEST(arch, API_BASELINE, label, NULL, "baseline-" name, \ - NULL, 0, NULL, flags, result); \ + DO_TEST(arch, cpuTestBaseline, label, NULL, \ + "baseline-" name, NULL, 0, NULL, flags, result); \ } \ VIR_FREE(label); \ } while (0) #define DO_TEST_HASFEATURE(arch, host, feature, result) \ - DO_TEST(arch, API_HAS_FEATURE, \ + DO_TEST(arch, cpuTestHasFeature, \ host "/" feature " (" #result ")", \ host, feature, NULL, 0, NULL, 0, result) #define DO_TEST_GUESTDATA(arch, host, cpu, models, preferred, result) \ - DO_TEST(arch, API_GUEST_DATA, \ + DO_TEST(arch, cpuTestGuestData, \ host "/" cpu " (" #models ", pref=" #preferred ")", \ host, cpu, models, \ models == NULL ? 0 : sizeof(models) / sizeof(char *), \ @@ -695,7 +663,7 @@ mymain(void) # define DO_TEST_CPUID_JSON(arch, host, json) \ do { \ if (json) { \ - DO_TEST(arch, API_JSON_CPUID, host, host, \ + DO_TEST(arch, cpuTestJSONCPUID, host, host, \ NULL, NULL, 0, NULL, 0, 0); \ } \ } while (0) @@ -705,9 +673,9 @@ mymain(void) #define DO_TEST_CPUID(arch, host, json) \ do { \ - DO_TEST(arch, API_HOST_CPUID, host, host, \ + DO_TEST(arch, cpuTestHostCPUID, host, host, \ NULL, NULL, 0, NULL, 0, 0); \ - DO_TEST(arch, API_GUEST_CPUID, host, host, \ + DO_TEST(arch, cpuTestGuestCPUID, host, host, \ NULL, NULL, 0, NULL, 0, 0); \ DO_TEST_CPUID_JSON(arch, host, json); \ } while (0) -- 2.8.4

2 1

[libvirt] [PATCH] Fix build without xen
by Martin Kletzander 14 Jun '16

14 Jun '16

Commit 11567cf66f36 introduced an include which will only work when building with xen (particularly libxl). However, that file is supposed to be includable from anywhere (as with other testutils* files. Signed-off-by: Martin Kletzander <mkletzan(a)redhat.com> --- Pushed under the build-breaker rule. tests/testutilsxen.h | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/tests/testutilsxen.h b/tests/testutilsxen.h index 8b997c32325c..2facf5e5afa5 100644 --- a/tests/testutilsxen.h +++ b/tests/testutilsxen.h @@ -2,7 +2,9 @@ # define _TESTUTILSXEN_H_ # include "capabilities.h" -# include "libxl/libxl_capabilities.h" +# ifdef WITH_LIBXL +# include "libxl/libxl_capabilities.h" +# endif virCapsPtr testXenCapsInit(void); -- 2.8.4

1 0

[libvirt] [PATCH] virt-host-validate: fix build with clang
by Roman Bogorodskiy 14 Jun '16

14 Jun '16

Building with clang 3.8 triggers the following error: CC virt_host_validate-virt-host-validate-qemu.o virt-host-validate-qemu.c:36:11: error: initializing 'char *' with an expression of type 'const char *' discards qualifiers [-Werror,-Wincompatible-pointer-types-discards-qualifiers] char *kvmhint = _("Check that CPU and firmware supports virtualization " ^ ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ virt-host-validate-qemu.c:46:17: error: assigning to 'char *' from 'const char *' discards qualifiers [-Werror,-Wincompatible-pointer-types-discards-qualifiers] kvmhint = _("Check that the 'kvm-intel' or 'kvm-amd' modules are " ^ ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ 2 errors generated. Fix by using 'const' for kvmhint declaration. --- Pushed under the build-breaker rule. tools/virt-host-validate-qemu.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tools/virt-host-validate-qemu.c b/tools/virt-host-validate-qemu.c index 66377e1..2aa396e 100644 --- a/tools/virt-host-validate-qemu.c +++ b/tools/virt-host-validate-qemu.c @@ -33,8 +33,8 @@ int virHostValidateQEMU(void) int ret = 0; bool hasHwVirt = false; bool hasVirtFlag = false; - char *kvmhint = _("Check that CPU and firmware supports virtualization " - "and kvm module is loaded"); + const char *kvmhint = _("Check that CPU and firmware supports virtualization " + "and kvm module is loaded"); if (!(flags = virHostValidateGetCPUFlags())) return -1; -- 2.7.4

1 0

[libvirt] [PATCH v5 0/2] Add support for zero-write detection
by Martin Kletzander 13 Jun '16

13 Jun '16

v5: - Reword documentation - Use detect_zeroes=on if it is unmap but discard=ignore - Use detect-zeroes on qemu cmdline instead of detect_zeroes v4: - Changed docs per Peter's review - https://www.redhat.com/archives/libvir-list/2016-June/msg00113.html v3: - I know Peter said he'll review it, but there are conflicts every once in a while, so rebased version will apply cleanly for a while - And it's actually not that big of a change to wait for some bigger things to be designed, I believe =) - https://www.redhat.com/archives/libvir-list/2016-April/msg01926.html v2: - use 'zeroes' instead of 'zeros' - https://www.redhat.com/archives/libvir-list/2016-February/msg01146.html v1: - https://www.redhat.com/archives/libvir-list/2015-December/msg00511.html Martin Kletzander (2): conf: Add support of zero-detection for disks qemu: Add support for zero-detection writes docs/formatdomain.html.in | 12 ++++++ docs/schemas/domaincommon.rng | 12 ++++++ src/conf/domain_conf.c | 19 ++++++++- src/conf/domain_conf.h | 11 ++++++ src/libvirt_private.syms | 2 + src/qemu/qemu_capabilities.c | 2 + src/qemu/qemu_capabilities.h | 1 + src/qemu/qemu_command.c | 24 ++++++++++++ tests/qemucapabilitiesdata/caps_2.1.1.x86_64.xml | 1 + tests/qemucapabilitiesdata/caps_2.4.0.x86_64.xml | 1 + tests/qemucapabilitiesdata/caps_2.5.0.x86_64.xml | 1 + .../caps_2.6.0-gicv2.aarch64.xml | 1 + .../caps_2.6.0-gicv3.aarch64.xml | 1 + tests/qemucapabilitiesdata/caps_2.6.0.ppc64le.xml | 1 + tests/qemucapabilitiesdata/caps_2.6.0.x86_64.xml | 1 + .../qemuxml2argv-disk-drive-detect-zeroes.args | 27 +++++++++++++ .../qemuxml2argv-disk-drive-detect-zeroes.xml | 45 ++++++++++++++++++++++ tests/qemuxml2argvtest.c | 3 ++ .../qemuxml2xmlout-disk-drive-detect-zeroes.xml | 1 + tests/qemuxml2xmltest.c | 1 + 20 files changed, 166 insertions(+), 1 deletion(-) create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-disk-drive-detect-zeroes.args create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-disk-drive-detect-zeroes.xml create mode 120000 tests/qemuxml2xmloutdata/qemuxml2xmlout-disk-drive-detect-zeroes.xml -- 2.8.3

2 3

[libvirt] [ruby-libvirt 1/2] Add .gitignore
by Guido Günther 13 Jun '16

13 Jun '16

--- .gitignore | 6 ++++++ 1 file changed, 6 insertions(+) create mode 100644 .gitignore diff --git a/.gitignore b/.gitignore new file mode 100644 index 0000000..f033dcf --- /dev/null +++ b/.gitignore @@ -0,0 +1,6 @@ +ext/libvirt/extconf.h +ext/libvirt/_libvirt.so +ext/libvirt/Makefile +ext/libvirt/mkmf.log +*.o +*~ -- 2.8.1

2 2

[libvirt] [PATCH] libxl: fix vm lock overwritten bug
by Wang Yufei 13 Jun '16

13 Jun '16

In libxl driver we do virObjectRef in libxlDomainObjBeginJob, If virCondWaitUntil failed, it goes to error, do virObjectUnref, There's a chance that someone undefine the vm at the same time, and refs unref to zero, vm is freed in libxlDomainObjBeginJob. But the vm outside function is not Null, we do virObjectUnlock(vm). That's how we overwrite the vm memory after it's freed. I fix it. Signed-off-by: Wang Yufei <james.wangyufei(a)huawei.com> --- src/conf/virdomainobjlist.c | 29 ++++++++-- src/conf/virdomainobjlist.h | 2 + src/libvirt_private.syms | 1 + src/libxl/libxl_domain.c | 18 ++----- src/libxl/libxl_domain.h | 5 +- src/libxl/libxl_driver.c | 127 ++++++++++++++++---------------------------- src/libxl/libxl_migration.c | 14 ++--- 7 files changed, 86 insertions(+), 110 deletions(-) diff --git a/src/conf/virdomainobjlist.c b/src/conf/virdomainobjlist.c index 27590c8..485671e 100644 --- a/src/conf/virdomainobjlist.c +++ b/src/conf/virdomainobjlist.c @@ -112,24 +112,45 @@ static int virDomainObjListSearchID(const void *payload, return want; } - -virDomainObjPtr virDomainObjListFindByID(virDomainObjListPtr doms, - int id) +static virDomainObjPtr +virDomainObjListFindByIDInternal(virDomainObjListPtr doms, + int id, + bool ref) { virDomainObjPtr obj; virObjectLock(doms); obj = virHashSearch(doms->objs, virDomainObjListSearchID, &id); + if (ref) { + virObjectRef(obj); + virObjectUnlock(doms); + } if (obj) { virObjectLock(obj); if (obj->removing) { virObjectUnlock(obj); + if (ref) + virObjectUnref(obj); obj = NULL; } } - virObjectUnlock(doms); + if (!ref) + virObjectUnlock(doms); return obj; } +virDomainObjPtr +virDomainObjListFindByID(virDomainObjListPtr doms, + int id) +{ + return virDomainObjListFindByIDInternal(doms, id, false); +} + +virDomainObjPtr +virDomainObjListFindByIDRef(virDomainObjListPtr doms, + int id) +{ + return virDomainObjListFindByIDInternal(doms, id, true); +} static virDomainObjPtr virDomainObjListFindByUUIDInternal(virDomainObjListPtr doms, diff --git a/src/conf/virdomainobjlist.h b/src/conf/virdomainobjlist.h index c0f856c..3c59bd3 100644 --- a/src/conf/virdomainobjlist.h +++ b/src/conf/virdomainobjlist.h @@ -34,6 +34,8 @@ virDomainObjListPtr virDomainObjListNew(void); virDomainObjPtr virDomainObjListFindByID(virDomainObjListPtr doms, int id); +virDomainObjPtr virDomainObjListFindByIDRef(virDomainObjListPtr doms, + int id); virDomainObjPtr virDomainObjListFindByUUID(virDomainObjListPtr doms, const unsigned char *uuid); virDomainObjPtr virDomainObjListFindByUUIDRef(virDomainObjListPtr doms, diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index 85b9cd1..b42e2cd 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -888,6 +888,7 @@ virDomainObjListCollect; virDomainObjListConvert; virDomainObjListExport; virDomainObjListFindByID; +virDomainObjListFindByIDRef; virDomainObjListFindByName; virDomainObjListFindByUUID; virDomainObjListFindByUUIDRef; diff --git a/src/libxl/libxl_domain.c b/src/libxl/libxl_domain.c index 4a21f68..ec1ff51 100644 --- a/src/libxl/libxl_domain.c +++ b/src/libxl/libxl_domain.c @@ -123,8 +123,6 @@ libxlDomainObjBeginJob(libxlDriverPrivatePtr driver ATTRIBUTE_UNUSED, return -1; then = now + LIBXL_JOB_WAIT_TIME; - virObjectRef(obj); - while (priv->job.active) { VIR_DEBUG("Wait normal job condition for starting job: %s", libxlDomainJobTypeToString(job)); @@ -157,7 +155,6 @@ libxlDomainObjBeginJob(libxlDriverPrivatePtr driver ATTRIBUTE_UNUSED, virReportSystemError(errno, "%s", _("cannot acquire job mutex")); - virObjectUnref(obj); return -1; } @@ -171,7 +168,7 @@ libxlDomainObjBeginJob(libxlDriverPrivatePtr driver ATTRIBUTE_UNUSED, * non-zero, false if the reference count has dropped to zero * and obj is disposed. */ -bool +void libxlDomainObjEndJob(libxlDriverPrivatePtr driver ATTRIBUTE_UNUSED, virDomainObjPtr obj) { @@ -183,8 +180,6 @@ libxlDomainObjEndJob(libxlDriverPrivatePtr driver ATTRIBUTE_UNUSED, libxlDomainObjResetJob(priv); virCondSignal(&priv->job.cond); - - return virObjectUnref(obj); } int @@ -532,12 +527,10 @@ libxlDomainShutdownThread(void *opaque) } endjob: - if (!libxlDomainObjEndJob(driver, vm)) - vm = NULL; + libxlDomainObjEndJob(driver, vm); cleanup: - if (vm) - virObjectUnlock(vm); + virDomainObjEndAPI(&vm); if (dom_event) libxlDomainEventQueue(driver, dom_event); libxl_event_free(cfg->ctx, ev); @@ -570,7 +563,7 @@ libxlDomainEventHandler(void *data, VIR_LIBXL_EVENT_CONST libxl_event *event) if (xl_reason == LIBXL_SHUTDOWN_REASON_SUSPEND) goto error; - vm = virDomainObjListFindByID(driver->domains, event->domid); + vm = virDomainObjListFindByIDRef(driver->domains, event->domid); if (!vm) { VIR_INFO("Received event for unknown domain ID %d", event->domid); goto error; @@ -605,8 +598,7 @@ libxlDomainEventHandler(void *data, VIR_LIBXL_EVENT_CONST libxl_event *event) /* Cast away any const */ libxl_event_free(cfg->ctx, (libxl_event *)event); virObjectUnref(cfg); - if (vm) - virObjectUnlock(vm); + virDomainObjEndAPI(&vm); VIR_FREE(shutdown_info); } diff --git a/src/libxl/libxl_domain.h b/src/libxl/libxl_domain.h index c53adaa..af11a2c 100644 --- a/src/libxl/libxl_domain.h +++ b/src/libxl/libxl_domain.h @@ -85,10 +85,9 @@ libxlDomainObjBeginJob(libxlDriverPrivatePtr driver, enum libxlDomainJob job) ATTRIBUTE_RETURN_CHECK; -bool +void libxlDomainObjEndJob(libxlDriverPrivatePtr driver, - virDomainObjPtr obj) - ATTRIBUTE_RETURN_CHECK; + virDomainObjPtr obj); int libxlDomainJobUpdateTime(struct libxlDomainJobObj *job) diff --git a/src/libxl/libxl_driver.c b/src/libxl/libxl_driver.c index 2ab08ac..629788d 100644 --- a/src/libxl/libxl_driver.c +++ b/src/libxl/libxl_driver.c @@ -290,7 +290,7 @@ libxlDomObjFromDomain(virDomainPtr dom) libxlDriverPrivatePtr driver = dom->conn->privateData; char uuidstr[VIR_UUID_STRING_BUFLEN]; - vm = virDomainObjListFindByUUID(driver->domains, dom->uuid); + vm = virDomainObjListFindByUUIDRef(driver->domains, dom->uuid); if (!vm) { virUUIDFormat(dom->uuid, uuidstr); virReportError(VIR_ERR_NO_DOMAIN, @@ -309,12 +309,12 @@ libxlAutostartDomain(virDomainObjPtr vm, libxlDriverPrivatePtr driver = opaque; int ret = -1; + virObjectRef(vm); virObjectLock(vm); virResetLastError(); if (libxlDomainObjBeginJob(driver, vm, LIBXL_JOB_MODIFY) < 0) { - virObjectUnlock(vm); - return ret; + goto cleanup; } if (vm->autostart && !virDomainObjIsActive(vm) && @@ -328,8 +328,9 @@ libxlAutostartDomain(virDomainObjPtr vm, ret = 0; endjob: - if (libxlDomainObjEndJob(driver, vm)) - virObjectUnlock(vm); + libxlDomainObjEndJob(driver, vm); + cleanup: + virDomainObjEndAPI(&vm); return ret; } @@ -983,6 +984,7 @@ libxlDomainCreateXML(virConnectPtr conn, const char *xml, VIR_DOMAIN_OBJ_LIST_ADD_CHECK_LIVE, NULL))) goto cleanup; + virObjectRef(vm); def = NULL; if (libxlDomainObjBeginJob(driver, vm, LIBXL_JOB_MODIFY) < 0) { @@ -1008,13 +1010,11 @@ libxlDomainCreateXML(virConnectPtr conn, const char *xml, dom->id = vm->def->id; endjob: - if (!libxlDomainObjEndJob(driver, vm)) - vm = NULL; + libxlDomainObjEndJob(driver, vm); cleanup: virDomainDefFree(def); - if (vm) - virObjectUnlock(vm); + virDomainObjEndAPI(&vm); virObjectUnref(cfg); return dom; } @@ -1141,12 +1141,10 @@ libxlDomainSuspend(virDomainPtr dom) ret = 0; endjob: - if (!libxlDomainObjEndJob(driver, vm)) - vm = NULL; + libxlDomainObjEndJob(driver, vm); cleanup: - if (vm) - virObjectUnlock(vm); + virDomainObjEndAPI(&vm); if (event) libxlDomainEventQueue(driver, event); virObjectUnref(cfg); @@ -1200,12 +1198,10 @@ libxlDomainResume(virDomainPtr dom) ret = 0; endjob: - if (!libxlDomainObjEndJob(driver, vm)) - vm = NULL; + libxlDomainObjEndJob(driver, vm); cleanup: - if (vm) - virObjectUnlock(vm); + virDomainObjEndAPI(&vm); if (event) libxlDomainEventQueue(driver, event); virObjectUnref(cfg); @@ -1373,12 +1369,10 @@ libxlDomainDestroyFlags(virDomainPtr dom, ret = 0; endjob: - if (!libxlDomainObjEndJob(driver, vm)) - vm = NULL; + libxlDomainObjEndJob(driver, vm); cleanup: - if (vm) - virObjectUnlock(vm); + virDomainObjEndAPI(&vm); if (event) libxlDomainEventQueue(driver, event); virObjectUnref(cfg); @@ -1517,12 +1511,10 @@ libxlDomainSetMemoryFlags(virDomainPtr dom, unsigned long newmem, ret = 0; endjob: - if (!libxlDomainObjEndJob(driver, vm)) - vm = NULL; + libxlDomainObjEndJob(driver, vm); cleanup: - if (vm) - virObjectUnlock(vm); + virDomainObjEndAPI(&vm); virObjectUnref(cfg); return ret; } @@ -1746,16 +1738,14 @@ libxlDomainSaveFlags(virDomainPtr dom, const char *to, const char *dxml, ret = 0; endjob: - if (!libxlDomainObjEndJob(driver, vm)) - vm = NULL; + libxlDomainObjEndJob(driver, vm); cleanup: if (remove_dom && vm) { virDomainObjListRemove(driver->domains, vm); vm = NULL; } - if (vm) - virObjectUnlock(vm); + virDomainObjEndAPI(&vm); return ret; } @@ -1802,7 +1792,7 @@ libxlDomainRestoreFlags(virConnectPtr conn, const char *from, VIR_DOMAIN_OBJ_LIST_ADD_CHECK_LIVE, NULL))) goto cleanup; - + virObjectRef(vm); def = NULL; if (libxlDomainObjBeginJob(driver, vm, LIBXL_JOB_MODIFY) < 0) { @@ -1819,15 +1809,13 @@ libxlDomainRestoreFlags(virConnectPtr conn, const char *from, if (ret < 0 && !vm->persistent) virDomainObjListRemove(driver->domains, vm); - if (!libxlDomainObjEndJob(driver, vm)) - vm = NULL; + libxlDomainObjEndJob(driver, vm); cleanup: if (VIR_CLOSE(fd) < 0) virReportSystemError(errno, "%s", _("cannot close file")); virDomainDefFree(def); - if (vm) - virObjectUnlock(vm); + virDomainObjEndAPI(&vm); virObjectUnref(cfg); return ret; } @@ -1923,16 +1911,14 @@ libxlDomainCoreDump(virDomainPtr dom, const char *to, unsigned int flags) } endjob: - if (!libxlDomainObjEndJob(driver, vm)) - vm = NULL; + libxlDomainObjEndJob(driver, vm); cleanup: if (remove_dom && vm) { virDomainObjListRemove(driver->domains, vm); vm = NULL; } - if (vm) - virObjectUnlock(vm); + virDomainObjEndAPI(&vm); if (event) libxlDomainEventQueue(driver, event); virObjectUnref(cfg); @@ -1986,16 +1972,14 @@ libxlDomainManagedSave(virDomainPtr dom, unsigned int flags) ret = 0; endjob: - if (!libxlDomainObjEndJob(driver, vm)) - vm = NULL; + libxlDomainObjEndJob(driver, vm); cleanup: if (remove_dom && vm) { virDomainObjListRemove(driver->domains, vm); vm = NULL; } - if (vm) - virObjectUnlock(vm); + virDomainObjEndAPI(&vm); VIR_FREE(name); return ret; } @@ -2212,14 +2196,12 @@ libxlDomainSetVcpusFlags(virDomainPtr dom, unsigned int nvcpus, } endjob: - if (!libxlDomainObjEndJob(driver, vm)) - vm = NULL; + libxlDomainObjEndJob(driver, vm); cleanup: VIR_FREE(bitmask); - if (vm) - virObjectUnlock(vm); - virObjectUnref(cfg); + virDomainObjEndAPI(&vm); + virObjectUnref(cfg); return ret; } @@ -2357,12 +2339,10 @@ libxlDomainPinVcpuFlags(virDomainPtr dom, unsigned int vcpu, } endjob: - if (!libxlDomainObjEndJob(driver, vm)) - vm = NULL; + libxlDomainObjEndJob(driver, vm); cleanup: - if (vm) - virObjectUnlock(vm); + virDomainObjEndAPI(&vm); virBitmapFree(pcpumap); virObjectUnref(cfg); return ret; @@ -2685,12 +2665,10 @@ libxlDomainCreateWithFlags(virDomainPtr dom, dom->id = vm->def->id; endjob: - if (!libxlDomainObjEndJob(driver, vm)) - vm = NULL; + libxlDomainObjEndJob(driver, vm); cleanup: - if (vm) - virObjectUnlock(vm); + virDomainObjEndAPI(&vm); return ret; } @@ -3695,14 +3673,12 @@ libxlDomainAttachDeviceFlags(virDomainPtr dom, const char *xml, } endjob: - if (!libxlDomainObjEndJob(driver, vm)) - vm = NULL; + libxlDomainObjEndJob(driver, vm); cleanup: virDomainDefFree(vmdef); virDomainDeviceDefFree(dev); - if (vm) - virObjectUnlock(vm); + virDomainObjEndAPI(&vm); virObjectUnref(cfg); return ret; } @@ -3788,14 +3764,12 @@ libxlDomainDetachDeviceFlags(virDomainPtr dom, const char *xml, } endjob: - if (!libxlDomainObjEndJob(driver, vm)) - vm = NULL; + libxlDomainObjEndJob(driver, vm); cleanup: virDomainDefFree(vmdef); virDomainDeviceDefFree(dev); - if (vm) - virObjectUnlock(vm); + virDomainObjEndAPI(&vm); virObjectUnref(cfg); return ret; } @@ -4076,14 +4050,12 @@ libxlDomainSetAutostart(virDomainPtr dom, int autostart) ret = 0; endjob: - if (!libxlDomainObjEndJob(driver, vm)) - vm = NULL; + libxlDomainObjEndJob(driver, vm); cleanup: VIR_FREE(configFile); VIR_FREE(autostartLink); - if (vm) - virObjectUnlock(vm); + virDomainObjEndAPI(&vm); virObjectUnref(cfg); return ret; } @@ -4288,12 +4260,10 @@ libxlDomainSetSchedulerParametersFlags(virDomainPtr dom, ret = 0; endjob: - if (!libxlDomainObjEndJob(driver, vm)) - vm = NULL; + libxlDomainObjEndJob(driver, vm); cleanup: - if (vm) - virObjectUnlock(vm); + virDomainObjEndAPI(&vm); virObjectUnref(cfg); return ret; } @@ -4609,12 +4579,10 @@ libxlDomainInterfaceStats(virDomainPtr dom, _("'%s' is not a known interface"), path); endjob: - if (!libxlDomainObjEndJob(driver, vm)) - vm = NULL; + libxlDomainObjEndJob(driver, vm); cleanup: - if (vm) - virObjectUnlock(vm); + virDomainObjEndAPI(&vm); return ret; } @@ -4791,13 +4759,11 @@ libxlDomainMemoryStats(virDomainPtr dom, ret = i; endjob: - if (!libxlDomainObjEndJob(driver, vm)) - vm = NULL; + libxlDomainObjEndJob(driver, vm); cleanup: libxl_dominfo_dispose(&d_info); - if (vm) - virObjectUnlock(vm); + virDomainObjEndAPI(&vm); virObjectUnref(cfg); return ret; } @@ -5364,8 +5330,7 @@ libxlDomainMigrateFinish3Params(virConnectPtr dconn, ret = libxlDomainMigrationFinish(dconn, vm, flags, cancelled); - if (!libxlDomainObjEndJob(driver, vm)) - vm = NULL; + libxlDomainObjEndJob(driver, vm); virDomainObjEndAPI(&vm); diff --git a/src/libxl/libxl_migration.c b/src/libxl/libxl_migration.c index 539ee1b..bf285a4 100644 --- a/src/libxl/libxl_migration.c +++ b/src/libxl/libxl_migration.c @@ -266,6 +266,7 @@ libxlDoMigrateReceive(void *opaque) int ret; bool remove_dom = 0; + virObjectRef(vm); virObjectLock(vm); if (libxlDomainObjBeginJob(driver, vm, LIBXL_JOB_MODIFY) < 0) goto cleanup; @@ -291,16 +292,14 @@ libxlDoMigrateReceive(void *opaque) VIR_FORCE_CLOSE(recvfd); virObjectUnref(args); - if (!libxlDomainObjEndJob(driver, vm)) - vm = NULL; + libxlDomainObjEndJob(driver, vm); cleanup: if (remove_dom && vm) { virDomainObjListRemove(driver->domains, vm); vm = NULL; } - if (vm) - virObjectUnlock(vm); + virDomainObjEndAPI(&vm); } @@ -437,14 +436,11 @@ libxlDomainMigrationBegin(virConnectPtr conn, xml = virDomainDefFormat(def, cfg->caps, VIR_DOMAIN_DEF_FORMAT_SECURE); endjob: - if (!libxlDomainObjEndJob(driver, vm)) - vm = NULL; + libxlDomainObjEndJob(driver, vm); cleanup: libxlMigrationCookieFree(mig); - if (vm) - virObjectUnlock(vm); - + virDomainObjEndAPI(&vm); virDomainDefFree(tmpdef); virObjectUnref(cfg); return xml; -- 1.8.3.4

2 1

[libvirt] [GSoC] Abstracting device address allocation
by Tomasz Flendrich 13 Jun '16

13 Jun '16

Hello everyone, Let me introduce myself - I'm Tomasz Flendrich and I'm a Google Summer of Code student from University of Wrocław, Poland. My goal is to create a generic address allocator (for PCI, virtio, SCSI, etc.), because current handling of addresses has its flaws: sometimes addresses aren't properly assigned and checked for duplicates. Additionally, a generic solution will be very useful when more hypervisors add the feature of explicitly stating the addresses of devices. The key goal I'm willing to achieve at this moment is defining a minimum viable product (one thing to be implemented quickly which is reasonably complete on its own, and has potential for further development in the direction which makes sense). I came up with an initial design. The internal allocator's data will be kept in struct Allocator. There will be one structure for one address type and all of the allocators would be in an array. We will have the possibility of adding an address pool to the allocator. For example, when we create a root PCI bus, we tell the allocator that the following PCI addresses are possible: {0..0}:{0..31}.{0..7}, where {a..b} means a range from a to b, inclusive. This function call could look like this: allocatorAddPool( allocator[PCI_ADDRESS_TYPE], &outputAddress, AllocatorRange(0, 0), AllocatorRange(0, 31), AllocatorRange(0, 7)); The outputAddress would be an array owned by the caller (only filled in by the allocator). If we were to allocate IP addresses for computers from the range 192.168.1.2 to 192.168.1.240, where the router has to have the address 192.168.1.254: First, we reserve the address for the router to let the Allocator know that it's in use, and that we can track collisions in manually assigned addresses: allocatorReserveAddress( allocator[IP_ADDRESS_TYPE], &outputAddress, AllocatorValue(192), AllocatorValue(168), AllocatorValue(1), AllocatorValue(254)); Then, we assign the addresses for the computers: allocatorReserveAddress( allocator[IP_ADDRESS_TYPE], &outputAddress, AllocatorValue(192), AllocatorValue(168), AllocatorValue(1), AllocatorRange(1, 254)); Please note that AllocatorValue() is now in use. There could be a different function call to simply assign any address: allocatorReserveAny(Allocator* allocator, &outputAddress); Let's say that we want an "sda" disk. We could create a wrapper: allocatorReserveSCSIAddress(allocator, "sda"); All this wrapper does is representing the string "sda" as an integer mapping to the letter (eg. 'a' = 0, 'z' = 25): allocatorReserveAddress(allocator[SCSI_ADDRESS_TYPE], &outputAddress, 0); If an address is already determined, because it was specified in the XML or it's some specific device that has to be at some specific address, we still reserve it to let the Allocator know that it's in use. How would this work internally? One of the possible solutions is keeping a set of ranges of addresses. For example, suppose that we have two PCI busses to use. Our address pool is stored as one range: {0..1} {0..31} {0..7} Now someone reserves address 0:13.2 Our new free addresses pool is now stored as this set of ranges: {0..0} {0..12} {0..7}, {0..0} {12..12} {0..1}, {0..0} {12..12} {3..7}, {0..0} {13..31} {0..7}, {1..1} {0..31} {0..7} If we kept every address separately, it would require 2*32*8=512 addresses. The set data structure from gnulib's gl_oset.h is a candidate for keeping the ranges in a sorted fashion. Another candidate is simply a sorted list from gl_list.h. This structure would be able to handle all types of addresses that are convertible to a fixed-length list of integers. We don't mind how many of these integers there are, because we can use variadic arguments. It won't allow duplicates if we stick to using it in every place where we have some addresses. It will also be very generic, with the possibility of writing wrappers on top of it that might be more convenient for some particular address type. This way we would keep qemu_* files as they are for now, and just replace existing allocators (and manual assignment of addresses) to calls to our new allocator. My ideas: - A possibility of reserving a range of addresses might be used to reserve a whole PCI slot at once. - Flags could be added, changing the behavior of particular addresses or the whole allocator. My questions: - Is this approach good long-term? - Is address releasing expected to be required at any point? (think device hotplug/unplug, etc.) - What other features are needed? Please speak up your mind if you have remarks or thoughts about the idea. I'd really appreciate it. Tomasz

3 5