[libvirt] [PATCH v2 0/3] Miscellaneous cleanups
This is basically v2 of long lost [1] with fixes for what Osier found out and with one one-liner patch added. [1] http://www.redhat.com/archives/libvir-list/2012-December/msg01043.html Martin Kletzander (3): conf: Don't format cputune element when not needed docs: aesthetical cleanups Ignore '.trs' files .gitignore | 1 + HACKING | 10 +++++----- docs/firewall.html.in | 2 +- docs/formatdomain.html.in | 4 ++-- docs/hacking.html.in | 10 +++++----- src/conf/domain_conf.c | 12 ++++++------ src/locking/lock_driver.h | 6 +++--- 7 files changed, 23 insertions(+), 22 deletions(-) -- 1.8.1.2
Commit 60b176c3d0f0d5037acfa5e27c7753f657833a0b introduced a bug that when editing an XML with cputune similar to this: ... <vcpu placement='static' current='1'>2</vcpu> <cputune> <vcpupin vcpu="1" cpuset="0"/> </cputune> ... results in formatted XML that looks like this: ... <vcpu placement='static' current='1'>2</vcpu> <cputune> </cputune> ... That is caused by a condition depending on def->cputune.vcpupin being set rather than checking def->cputune.nvcpupin. Notice that nvcpupin can be 0 and vcpupin can still be allocated since it's a pointer to an array, so no harm done there. I also changed it on other places in the code where it depended on the wrong variable. --- v2: - I checked other places that used the false presumption, found one and fixed it as well --- src/conf/domain_conf.c | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 8dbfb96..ac4b2c2 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -13955,10 +13955,10 @@ virDomainIsAllVcpupinInherited(virDomainDefPtr def) int i; if (!def->cpumask) { - if (!def->cputune.vcpupin) - return true; - else + if (def->cputune.nvcpupin) return false; + else + return true; } else { for (i = 0; i < def->cputune.nvcpupin; i++) { if (!virBitmapEqual(def->cputune.vcpupin[i]->cpumask, @@ -14143,7 +14143,7 @@ virDomainDefFormatInternal(virDomainDefPtr def, virBufferAsprintf(buf, ">%u</vcpu>\n", def->maxvcpus); if (def->cputune.shares || - (def->cputune.vcpupin && !virDomainIsAllVcpupinInherited(def)) || + (def->cputune.nvcpupin && !virDomainIsAllVcpupinInherited(def)) || def->cputune.period || def->cputune.quota || def->cputune.emulatorpin || def->cputune.emulator_period || def->cputune.emulator_quota) @@ -14209,7 +14209,7 @@ virDomainDefFormatInternal(virDomainDefPtr def, VIR_FREE(cpumask); } if (def->cputune.shares || - (def->cputune.vcpupin && !virDomainIsAllVcpupinInherited(def)) || + (def->cputune.nvcpupin && !virDomainIsAllVcpupinInherited(def)) || def->cputune.period || def->cputune.quota || def->cputune.emulatorpin || def->cputune.emulator_period || def->cputune.emulator_quota) -- 1.8.1.2
Adding dots inside "exempli gratia" where missing. While on that, I took the liberty of changing it where found with simple grep. --- v2: - I git grep'd the whole repo and fixed it everywhere, not just in HACKING --- HACKING | 10 +++++----- docs/firewall.html.in | 2 +- docs/formatdomain.html.in | 4 ++-- docs/hacking.html.in | 10 +++++----- src/conf/domain_conf.c | 2 +- src/locking/lock_driver.h | 6 +++--- 6 files changed, 17 insertions(+), 17 deletions(-) diff --git a/HACKING b/HACKING index 3b800d8..c7df3f3 100644 --- a/HACKING +++ b/HACKING @@ -207,29 +207,29 @@ declare them at the beginning of a scope, rather than immediately before use. Bracket spacing =============== The keywords "if", "for", "while", and "switch" must have a single space -following them before the opening bracket. eg +following them before the opening bracket. E.g. if(foo) // Bad if (foo) // Good -Function implementations mustnothave any whitespace between the function name and the opening bracket. eg +Function implementations mustnothave any whitespace between the function name and the opening bracket. E.g. int foo (int wizz) // Bad int foo(int wizz) // Good -Function calls mustnothave any whitespace between the function name and the opening bracket. eg +Function calls mustnothave any whitespace between the function name and the opening bracket. E.g. bar = foo (wizz); // Bad bar = foo(wizz); // Good Function typedefs mustnothave any whitespace between the closing bracket of the function name and -opening bracket of the arg list. eg +opening bracket of the arg list. E.g. typedef int (*foo) (int wizz); // Bad typedef int (*foo)(int wizz); // Good There must not be any whitespace immediately following any opening bracket, or -immediately prior to any closing bracket +immediately prior to any closing bracket. E.g. int foo( int wizz ); // Bad int foo(int wizz); // Good diff --git a/docs/firewall.html.in b/docs/firewall.html.in index afc8dd3..a9af704 100644 --- a/docs/firewall.html.in +++ b/docs/firewall.html.in @@ -198,7 +198,7 @@ using an XML format. At a high level the format looks like this: </p> <p>The <code><rule></code> element is where all the interesting stuff happens. It has three attributes, an action, a traffic direction and an - optional priority. eg: + optional priority. E.g.: </p> <pre><rule action='drop' direction='out' priority='500'></pre> <p>Within the rule there are a wide variety of elements allowed, which diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in index 7ad8aea..c01f564 100644 --- a/docs/formatdomain.html.in +++ b/docs/formatdomain.html.in @@ -1492,8 +1492,8 @@ attribute specifies the type of disk device to emulate; possible values are driver specific, with typical values being "ide", "scsi", "virtio", "xen", "usb" or "sata". If omitted, the bus - type is inferred from the style of the device name. eg, a device named - 'sda' will typically be exported using a SCSI bus. The optional + type is inferred from the style of the device name (e.g. a device named + 'sda' will typically be exported using a SCSI bus). The optional attribute <code>tray</code> indicates the tray status of the removable disks (i.e. CDROM or Floppy disk), the value can be either "open" or "closed", defaults to "closed". NB, the value of diff --git a/docs/hacking.html.in b/docs/hacking.html.in index caec01a..78620fc 100644 --- a/docs/hacking.html.in +++ b/docs/hacking.html.in @@ -241,7 +241,7 @@ <p> The keywords <code>if</code>, <code>for</code>, <code>while</code>, and <code>switch</code> must have a single space following them - before the opening bracket. eg + before the opening bracket. e.g. </p> <pre> if(foo) // Bad @@ -250,7 +250,7 @@ <p> Function implementations must <strong>not</strong> have any whitespace - between the function name and the opening bracket. eg + between the function name and the opening bracket. e.g. </p> <pre> int foo (int wizz) // Bad @@ -259,7 +259,7 @@ <p> Function calls must <strong>not</strong> have any whitespace - between the function name and the opening bracket. eg + between the function name and the opening bracket. e.g. </p> <pre> bar = foo (wizz); // Bad @@ -269,7 +269,7 @@ <p> Function typedefs must <strong>not</strong> have any whitespace between the closing bracket of the function name and opening - bracket of the arg list. eg + bracket of the arg list. e.g. </p> <pre> typedef int (*foo) (int wizz); // Bad @@ -278,7 +278,7 @@ <p> There must not be any whitespace immediately following any - opening bracket, or immediately prior to any closing bracket + opening bracket, or immediately prior to any closing bracket. e.g. </p> <pre> int foo( int wizz ); // Bad diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index ac4b2c2..abf2b6b 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -3769,7 +3769,7 @@ virDomainDiskDefParseXML(virCapsPtr caps, /* People sometimes pass a bogus '' source path when they mean to omit the source element - completely. eg CDROM without media. This is + completely (e.g. CDROM without media). This is just a little compatibility check to help those broken apps */ if (source && STREQ(source, "")) diff --git a/src/locking/lock_driver.h b/src/locking/lock_driver.h index e8ee226..c4b162c 100644 --- a/src/locking/lock_driver.h +++ b/src/locking/lock_driver.h @@ -1,7 +1,7 @@ /* * lock_driver.h: Defines the lock driver plugin API * - * Copyright (C) 2010-2011 Red Hat, Inc. + * Copyright (C) 2010-2011, 2013 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 @@ -186,8 +186,8 @@ typedef void (*virLockDriverFree)(virLockManagerPtr man); * * Assign a resource to a managed object. This will * only be called prior to the object is being locked - * when it is inactive. eg, to set the initial boot - * time disk assignments on a VM + * when it is inactive (e.g. to set the initial boot + * time disk assignments on a VM). * The format of @name varies according to * the resource @type. A VIR_LOCK_MANAGER_RESOURCE_TYPE_DISK * will have the fully qualified file path, while a resource -- 1.8.1.2
When doing checks with automake, there are '<testname>.trs' files left behind, that might or might not be usable, however these show up in 'git status' even though we definitely don't want them to be tracked in the repository'. Automake adds the '--trs-files' option by default since commit 0c81b43f711fb861f04227ced8dba889596d9c43 [1], which consequently (from 1.13 in my case) started leaving these files behind along with '<testname>.log' files as well (which we already ignore). [1] http://git.savannah.gnu.org/gitweb/?p=automake.git;a=commitdiff;h=0c81b43 --- .gitignore | 1 + 1 file changed, 1 insertion(+) diff --git a/.gitignore b/.gitignore index 291d834..1670637 100644 --- a/.gitignore +++ b/.gitignore @@ -134,6 +134,7 @@ /src/virtlockd.init /tests/*.log /tests/*.pid +/tests/*.trs /tests/*xml2*test /tests/commandhelper /tests/commandtest -- 1.8.1.2
On 01/29/2013 09:12 AM, Martin Kletzander wrote:
This is basically v2 of long lost [1] with fixes for what Osier found out and with one one-liner patch added.
[1] http://www.redhat.com/archives/libvir-list/2012-December/msg01043.html
Martin Kletzander (3): conf: Don't format cputune element when not needed docs: aesthetical cleanups Ignore '.trs' files
ACK series, and all safe to apply before the release. -- Eric Blake eblake redhat com +1-919-301-3266 Libvirt virtualization library http://libvirt.org
On 01/29/2013 08:49 PM, Eric Blake wrote:
On 01/29/2013 09:12 AM, Martin Kletzander wrote:
This is basically v2 of long lost [1] with fixes for what Osier found out and with one one-liner patch added.
[1] http://www.redhat.com/archives/libvir-list/2012-December/msg01043.html
Martin Kletzander (3): conf: Don't format cputune element when not needed docs: aesthetical cleanups Ignore '.trs' files
ACK series, and all safe to apply before the release.
Thanks, pushed. I'm glad this will be cleaned up in the release. Martin
participants (2)
-
Eric Blake -
Martin Kletzander