- Devel - libvirt List Archives

[libvirt] [PATCH] Support CPU less NUMA node.
by Shaohe Feng 26 Apr '17

26 Apr '17

Some platform NUMA node does not include cpu, such as Phi. This patch support CPU less NUMA node. But there must be one CPU cell node for a guest. Also must assign the host nodeset for this guest cell node. please enable numactl with "--with-numactl" for libvirt config. Test this patch: 1. define a numa cell without "cpus", such as cell 1. <numatune> <memnode cellid='1' mode='strict' nodeset='0'/> </numatune> <cpu> <numa> <cell id='0' cpus='0-1' memory='512000' unit='KiB'/> <cell id='1' memory='512000' unit='KiB'/> </numa> </cpu> libvirt can edit and start the VM successfully. 2. define a numa cell without "cpus", without nodeset. <cpu> <numa> <cell id='0' cpus='0-1' memory='512000' unit='KiB'/> <cell id='1' memory='512000' unit='KiB'/> </numa> </cpu> This case, libvirt will failed to edit the CPUS. --- src/conf/domain_conf.c | 4 +++ src/conf/numa_conf.c | 89 ++++++++++++++++++++++++++++++++++--------------- src/conf/numa_conf.h | 2 ++ src/qemu/qemu_command.c | 12 ++++--- src/util/virbitmap.c | 10 ++++-- 5 files changed, 83 insertions(+), 34 deletions(-) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 705deb3..d6e5489 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -17330,6 +17330,10 @@ virDomainDefParseXML(xmlDocPtr xml, ctxt) < 0) goto error; + if (virDomainNumaCPULessCheck(def->numa) < 0){ + goto error; + } + if (virDomainNumatuneHasPlacementAuto(def->numa) && !def->cpumask && !virDomainDefHasVcpuPin(def) && !def->cputune.emulatorpin && diff --git a/src/conf/numa_conf.c b/src/conf/numa_conf.c index bfd3703..1b6f7ff 100644 --- a/src/conf/numa_conf.c +++ b/src/conf/numa_conf.c @@ -742,34 +742,31 @@ virDomainNumaDefCPUParseXML(virDomainNumaPtr def, goto cleanup; } - if (!(tmp = virXMLPropString(nodes[i], "cpus"))) { - virReportError(VIR_ERR_XML_ERROR, "%s", - _("Missing 'cpus' attribute in NUMA cell")); - goto cleanup; - } - - if (virBitmapParse(tmp, &def->mem_nodes[cur_cell].cpumask, - VIR_DOMAIN_CPUMASK_LEN) < 0) - goto cleanup; - - if (virBitmapIsAllClear(def->mem_nodes[cur_cell].cpumask)) { - virReportError(VIR_ERR_CONFIG_UNSUPPORTED, - _("NUMA cell %d has no vCPUs assigned"), cur_cell); - goto cleanup; - } - VIR_FREE(tmp); - - for (j = 0; j < n; j++) { - if (j == cur_cell || !def->mem_nodes[j].cpumask) - continue; + tmp = virXMLPropString(nodes[i], "cpus"); + if (tmp) { + if (virBitmapParse(tmp, &def->mem_nodes[cur_cell].cpumask, + VIR_DOMAIN_CPUMASK_LEN) < 0) + goto cleanup; - if (virBitmapOverlaps(def->mem_nodes[j].cpumask, - def->mem_nodes[cur_cell].cpumask)) { + if (virBitmapIsAllClear(def->mem_nodes[cur_cell].cpumask)) { virReportError(VIR_ERR_CONFIG_UNSUPPORTED, - _("NUMA cells %u and %zu have overlapping vCPU ids"), - cur_cell, j); + _("NUMA cell %d has no vCPUs assigned"), cur_cell); goto cleanup; } + VIR_FREE(tmp); + + for (j = 0; j < n; j++) { + if (j == cur_cell || !def->mem_nodes[j].cpumask) + continue; + + if (virBitmapOverlaps(def->mem_nodes[j].cpumask, + def->mem_nodes[cur_cell].cpumask)) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, + _("NUMA cells %u and %zu have overlapping vCPU ids"), + cur_cell, j); + goto cleanup; + } + } } ctxt->node = nodes[i]; @@ -808,6 +805,7 @@ virDomainNumaDefCPUFormat(virBufferPtr buf, char *cpustr; size_t ncells = virDomainNumaGetNodeCount(def); size_t i; + size_t ncpuless = 0; if (ncells == 0) return 0; @@ -817,12 +815,24 @@ virDomainNumaDefCPUFormat(virBufferPtr buf, for (i = 0; i < ncells; i++) { memAccess = virDomainNumaGetNodeMemoryAccessMode(def, i); - if (!(cpustr = virBitmapFormat(virDomainNumaGetNodeCpumask(def, i)))) - return -1; + cpustr = virBitmapFormat(virDomainNumaGetNodeCpumask(def, i)); virBufferAddLit(buf, "<cell"); virBufferAsprintf(buf, " id='%zu'", i); - virBufferAsprintf(buf, " cpus='%s'", cpustr); + if (cpustr && (*cpustr != 0)) + virBufferAsprintf(buf, " cpus='%s'", cpustr); + else { + /* Note, at present we only allow cpuless numa node with */ + /* nodeset assign. */ + ncpuless++; + if (!virDomainNumatuneNodeSpecified(def, i)) { + VIR_FREE(cpustr); + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, + _("CPUless NUMA node cellid %zu must be " + "specified node set."), i); + return -1; + } + } virBufferAsprintf(buf, " memory='%llu'", virDomainNumaGetNodeMemorySize(def, i)); virBufferAddLit(buf, " unit='KiB'"); @@ -835,6 +845,12 @@ virDomainNumaDefCPUFormat(virBufferPtr buf, virBufferAdjustIndent(buf, -2); virBufferAddLit(buf, "</numa>\n"); + if (ncpuless == ncells) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, + _("At lease one NUMA node should " + "include CPU element.")); + return -1; + } return 0; } @@ -861,6 +877,7 @@ virDomainNumaGetMaxCPUID(virDomainNumaPtr numa) int bit; bit = virBitmapLastSetBit(virDomainNumaGetNodeCpumask(numa, i)); + bit = (bit > 0) ? bit : 0; if (bit > ret) ret = bit; } @@ -973,3 +990,21 @@ virDomainNumaGetMemorySize(virDomainNumaPtr numa) return ret; } + +// NOTE, For some plateforms, there are some node without cpus, such as Phi. +// We just need do some check for these plateforms, we must assigned nodeset +// for CPU less node. +int +virDomainNumaCPULessCheck(virDomainNumaPtr numa) +{ + int ret = 0; + int i = 0; + for (i = 0; i < numa->nmem_nodes; i++){ + if (!numa->mem_nodes[i].cpumask && !numa->mem_nodes[i].nodeset) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, + _("Cell: %d is a cpuless numa, must specfiy the nodeset."), i); + return -1; + } + } + return ret; +} diff --git a/src/conf/numa_conf.h b/src/conf/numa_conf.h index b6a5354..1f711e6 100644 --- a/src/conf/numa_conf.h +++ b/src/conf/numa_conf.h @@ -155,5 +155,7 @@ int virDomainNumaDefCPUFormat(virBufferPtr buf, virDomainNumaPtr def); unsigned int virDomainNumaGetCPUCountTotal(virDomainNumaPtr numa); +int virDomainNumaCPULessCheck(virDomainNumaPtr numa); + #endif /* __NUMA_CONF_H__ */ diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index b2e76ca..b07ca66 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -7665,11 +7665,13 @@ qemuBuildNumaArgStr(virQEMUDriverConfigPtr cfg, virCommandAddArg(cmd, "-numa"); virBufferAsprintf(&buf, "node,nodeid=%zu", i); - for (tmpmask = cpumask; tmpmask; tmpmask = next) { - if ((next = strchr(tmpmask, ','))) - *(next++) = '\0'; - virBufferAddLit(&buf, ",cpus="); - virBufferAdd(&buf, tmpmask, -1); + if (*cpumask != 0) { + for (tmpmask = cpumask; tmpmask; tmpmask = next) { + if ((next = strchr(tmpmask, ','))) + *(next++) = '\0'; + virBufferAddLit(&buf, ",cpus="); + virBufferAdd(&buf, tmpmask, -1); + } } if (needBackend) diff --git a/src/util/virbitmap.c b/src/util/virbitmap.c index eac63d9..b2803ce 100644 --- a/src/util/virbitmap.c +++ b/src/util/virbitmap.c @@ -953,6 +953,9 @@ virBitmapLastSetBit(virBitmapPtr bitmap) unsigned long bits; /* If bitmap is empty then there is no set bit */ + if (!bitmap) + return -1; + if (bitmap->map_len == 0) return -1; @@ -1039,9 +1042,12 @@ virBitmapCountBits(virBitmapPtr bitmap) size_t i; size_t ret = 0; - for (i = 0; i < bitmap->map_len; i++) - ret += count_one_bits_l(bitmap->map[i]); + if (bitmap == NULL) + return ret; + for (i = 0; i < bitmap->map_len; i++) { + ret += count_one_bits_l(bitmap->map[i]); + } return ret; } -- 2.7.4

3 4

[libvirt] [PATCH] virsh-domain: update attach-interface to support bridge type
by ZhiPeng Lu 26 Apr '17

26 Apr '17

Adding this feature will allow users to attach a network to linux bridge or openvswitch. The interface can be attached using --brtype=linux or --brtype=openvswitch. If --brtype is not set, the network is attached to linux bridge. Signed-off-by: ZhiPeng Lu <lu.zhipeng(a)zte.com.cn> --- tools/virsh-domain.c | 23 +++++++++++++++++++++-- tools/virsh.pod | 3 +++ 2 files changed, 24 insertions(+), 2 deletions(-) diff --git a/tools/virsh-domain.c b/tools/virsh-domain.c index db8accf..2690689 100644 --- a/tools/virsh-domain.c +++ b/tools/virsh-domain.c @@ -766,6 +766,10 @@ static const vshCmdOptDef opts_attach_interface[] = { .type = VSH_OT_BOOL, .help = N_("libvirt will automatically detach/attach the device from/to host") }, + {.name = "brtype", + .type = VSH_OT_STRING, + .help = N_("Bridge type: linux / openvswitch") + }, {.name = NULL} }; @@ -819,7 +823,8 @@ cmdAttachInterface(vshControl *ctl, const vshCmd *cmd) virDomainPtr dom = NULL; const char *mac = NULL, *target = NULL, *script = NULL, *type = NULL, *source = NULL, *model = NULL, - *inboundStr = NULL, *outboundStr = NULL; + *inboundStr = NULL, *outboundStr = NULL, + *brtype = NULL; virNetDevBandwidthRate inbound, outbound; virDomainNetType typ; int ret; @@ -850,7 +855,8 @@ cmdAttachInterface(vshControl *ctl, const vshCmd *cmd) vshCommandOptStringReq(ctl, cmd, "script", &script) < 0 || vshCommandOptStringReq(ctl, cmd, "model", &model) < 0 || vshCommandOptStringReq(ctl, cmd, "inbound", &inboundStr) < 0 || - vshCommandOptStringReq(ctl, cmd, "outbound", &outboundStr) < 0) + vshCommandOptStringReq(ctl, cmd, "outbound", &outboundStr) < 0 || + vshCommandOptStringReq(ctl, cmd, "brtype", &brtype) < 0) goto cleanup; /* check interface type */ @@ -946,6 +952,19 @@ cmdAttachInterface(vshControl *ctl, const vshCmd *cmd) if (model != NULL) virBufferAsprintf(&buf, "<model type='%s'/>\n", model); + if ((brtype != NULL)) { + if (STRNEQ(brtype , "linux") && STRNEQ(brtype , "openvswitch")) { + vshError(ctl, _("bridge type is incorrect, should be linux " + "or openvswitch")); + goto cleanup; + } + if (typ != VIR_DOMAIN_NET_TYPE_BRIDGE) { + vshError(ctl, _("No support for %s with bridge type"), type); + goto cleanup; + } + if (STREQ(brtype , "openvswitch")) + virBufferAsprintf(&buf, " <virtualport type='openvswitch'/>\n"); + } if (inboundStr || outboundStr) { virBufferAddLit(&buf, "<bandwidth>\n"); virBufferAdjustIndent(&buf, 2); diff --git a/tools/virsh.pod b/tools/virsh.pod index e16f62f..3cd0490 100644 --- a/tools/virsh.pod +++ b/tools/virsh.pod @@ -2935,6 +2935,9 @@ B<--managed> is usable only for I<hostdev> type and tells libvirt that the interface should be managed, which means detached and reattached from/to the host by libvirt. +B<--brtype> is usable only for I<bridge> type and tells libvirt +that the interface attached to bridge type (linux bridge or openvswitch). + If B<--print-xml> is specified, then the XML of the interface that would be attached is printed instead. -- 1.8.3.1

2 1

[libvirt] [PATCH 00/15] Multiple cleanups within nwfilterobj and nwfilter drivers
by John Ferlan 25 Apr '17

25 Apr '17

More adjustments in preparation for having virobject code handle the bulk of the object management code. I know it's a lot of patches, but reducing the amount of change overall has the opposite effect of increasing the number patches to show the work flow. Should make the "next" steps easier to review, but a couple of these will be tedious. John Ferlan (15): nwfilter: Use consistent naming for variables nwfilter: Use virNWFilterDefPtr rather than deref virNWFilterObjPtr nwfilter: Remove unused 'active' in virNWFilterObj nwfilter: Convert wantRemoved to bool nwfilter: Make _virNWFilterObjPtr private nwfilter: Introduce virNWFilterObjNew nwfilter: Rename some virNWFilterObj* API's nwfilter: Make _virNWFilterObjList private nwfilter: Make a common UUID lookup function from driver nwfilter: Replace virNWFilterConfigFile with virFileBuildPath nwfilter: Replace virNWFilterSaveDef with virNWFilterSaveConfig nwfilter: Move creation of configDir to driver initialization nwfilter: Move configFile name processing into driver nwfilter: Move save of config until after successful assign nwfilter: Move and rename virNWFilterSaveConfig src/conf/nwfilter_conf.c | 107 +----------- src/conf/nwfilter_conf.h | 16 +- src/conf/virnwfilterobj.c | 308 ++++++++++++++++++++++----------- src/conf/virnwfilterobj.h | 74 ++++---- src/libvirt_private.syms | 21 ++- src/nwfilter/nwfilter_driver.c | 148 ++++++++++------ src/nwfilter/nwfilter_gentech_driver.c | 42 +++-- 7 files changed, 377 insertions(+), 339 deletions(-) -- 2.9.3

2 33

[libvirt] [PATCH v3 0/2] Use the shared keycodemap database
by Daniel P. Berrange 25 Apr '17

25 Apr '17

Follow up to: v1: https://www.redhat.com/archives/libvir-list/2017-March/msg00070.html v2: https://www.redhat.com/archives/libvir-list/2017-March/msg00136.html Changed in v3: - Rebase onto new autogen.sh - Use VIR_KEYMAP_ENTRY_MAX constant more - Change style of make vars - Fix & verify distcheck Changed in v2: - Fixed make syntax-check errors - Auto-generate man pages listing value key codes for use with the virDomainSendKey API / virsh send-key command Daniel P. Berrange (2): util: switch over to use keycodemapdb GIT submodule Add ability to generate man page describing key code names & values .gitignore | 3 +- .gitmodules | 3 + libvirt.spec.in | 1 + mingw-libvirt.spec.in | 2 + src/Makefile.am | 86 ++++++-- src/keycodemapdb | 1 + src/util/keymaps.csv | 464 ------------------------------------------ src/util/virkeycode-mapgen.py | 97 --------- src/util/virkeycode.c | 94 ++++----- tests/virkeycodetest.c | 5 + tools/virsh.pod | 20 ++ 11 files changed, 153 insertions(+), 623 deletions(-) create mode 160000 src/keycodemapdb delete mode 100644 src/util/keymaps.csv delete mode 100755 src/util/virkeycode-mapgen.py -- 2.9.3

2 5

[libvirt] [PATCH 00/14] Multiple cleanups within secretobj and secret_driver
by John Ferlan 25 Apr '17

25 Apr '17

More adjustments in preparation for having virobject code handle the bulk of the object management code. I know it's a lot of patches, but most should be relatively simple - especially the first 10... The last 4 set up for a "common" way to handle the config directory space as well and could be a bit more controversial. Perhaps the most being patch 12 which attempts to perform the mkdir of the configDir during initialization rather than first "define" of secret. Although someone could also pick out their own most favorite/controversial and surprise me. John Ferlan (14): secret: Need to set data->error on VIR_ALLOC_N failure secret: Convert virsecretobjs.h to use "newer" formatting style secret: Make some virSecretObj* functions static secret: Have virSecretObjNew return locked object secret: Use consistent naming for variables secret: Use virSecretDefPtr rather than deref from virSecretObjPtr secret: Move virSecretObjListGetUUIDs secret: Change variable names for list traversals secret: Split apart NumOfSecrets and GetUUIDs callback function secret: Combine virSecretObjListAdd with Locked function secret: Alter FindByUUID to expect the formatted uuidstr secret: Generate configDir during driver initialization secret: Alter configFile/base64File mgmt secret: Clean up virSecretObjListExport logic src/conf/virsecretobj.c | 557 +++++++++++++++++++++++---------------------- src/conf/virsecretobj.h | 120 +++++----- src/secret/secret_driver.c | 178 ++++++++------- 3 files changed, 442 insertions(+), 413 deletions(-) -- 2.9.3

2 51

[libvirt] [PATCH v2 0/2] Augment debug messages.
by Prerna Saxena 25 Apr '17

25 Apr '17

This is a v2 of the previous set of debug enhancements posted at : https://www.redhat.com/archives/libvir-list/2017-March/msg00959.html Changelog: --------- 1) Patch 1/3 : virNetSocketGetFD() is reverted to its old state, and only new DEBUG messages are introduced. 2) Patch 2/3 : Dropped, per list suggestions. 3) Patch 3/3 : Same as v1 Prerna Saxena (2): Debug: Add DEBUG messages for when a client has opened/closed connection. Debug: Report VM errors more concisely. src/qemu/qemu_process.c | 4 ++-- src/rpc/virnetserverclient.c | 10 +++++++++- 2 files changed, 11 insertions(+), 3 deletions(-) -- 1.8.1.2

3 4

[libvirt] [PATCH v4 1/2] daemon: Fix domain name leak in error path
by Wang King 25 Apr '17

25 Apr '17

Domain name duplicated in make_nonnull_domain, but not freed when virTypedParamsSerialize return negative. --- daemon/remote.c | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/daemon/remote.c b/daemon/remote.c index 1610fea..a8c21fd 100644 --- a/daemon/remote.c +++ b/daemon/remote.c @@ -1066,8 +1066,10 @@ remoteRelayDomainEventTunable(virConnectPtr conn, if (virTypedParamsSerialize(params, nparams, (virTypedParameterRemotePtr *) &data.params.params_val, &data.params.params_len, - VIR_TYPED_PARAM_STRING_OKAY) < 0) + VIR_TYPED_PARAM_STRING_OKAY) < 0) { + VIR_FREE(data.dom.name); return -1; + } remoteDispatchObjectEventSend(callback->client, remoteProgram, REMOTE_PROC_DOMAIN_EVENT_CALLBACK_TUNABLE, @@ -1206,8 +1208,10 @@ remoteRelayDomainEventJobCompleted(virConnectPtr conn, if (virTypedParamsSerialize(params, nparams, (virTypedParameterRemotePtr *) &data.params.params_val, &data.params.params_len, - VIR_TYPED_PARAM_STRING_OKAY) < 0) + VIR_TYPED_PARAM_STRING_OKAY) < 0) { + VIR_FREE(data.dom.name); return -1; + } remoteDispatchObjectEventSend(callback->client, remoteProgram, REMOTE_PROC_DOMAIN_EVENT_CALLBACK_JOB_COMPLETED, -- 2.8.3

2 3

[libvirt] [PATCH] qemu: fix argument of virDomainNetGetActualDirectMode
by ZhiPeng Lu 25 Apr '17

25 Apr '17

it should be a comparison of modes between new and old devices. So the argument of the second virDomainNetGetActualDirectMode should be newdev. Signed-off-by: ZhiPeng Lu <lu.zhipeng(a)zte.com.cn> --- src/qemu/qemu_hotplug.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c index 120bcdc..eec99af 100644 --- a/src/qemu/qemu_hotplug.c +++ b/src/qemu/qemu_hotplug.c @@ -3273,7 +3273,7 @@ qemuDomainChangeNet(virQEMUDriverPtr driver, if (STRNEQ_NULLABLE(virDomainNetGetActualDirectDev(olddev), virDomainNetGetActualDirectDev(newdev)) || - virDomainNetGetActualDirectMode(olddev) != virDomainNetGetActualDirectMode(olddev) || + virDomainNetGetActualDirectMode(olddev) != virDomainNetGetActualDirectMode(newdev) || !virNetDevVPortProfileEqual(virDomainNetGetActualVirtPortProfile(olddev), virDomainNetGetActualVirtPortProfile(newdev)) || !virNetDevVlanEqual(virDomainNetGetActualVlan(olddev), -- 1.8.3.1

2 1

[libvirt] [PATCH v2] autogen.sh: Improve and generalize
by Andrea Bolognani 25 Apr '17

25 Apr '17

The goal is twofold: firstly, we want to extend the script so that it can deal with more than a single git submodule, and secondly we'd like to reduce the amount of duplicated code. Moreover, since we're making heavy changes to the code anyway, we might as well make sure it follows a somewhat consistent coding style too. To reduce code duplication, we introduce a new --dry-run option, which can be used by third parties to figure out whether calling autogen.sh is necessary or not: this allows us to get rid of the reimplementation of part of the logic in cfg.mk and guarantee they'll never get out of sync. Other changes include: making dirty submodules checking and cleaning entirely independent of other operations; removing the use of 'set -e' and handling errors explicitly instead; better parsing of command line arguments. --- Changes from [v1]: * drop the generic submodule update framework and simply update all submodules unconditionally, *then* perform any required gnulib-specific step; * handle "autogen not required" and "dry run error" differently in cfg.mk; * improve comments. [v1] https://www.redhat.com/archives/libvir-list/2017-April/msg00816.html autogen.sh | 253 ++++++++++++++++++++++++++++++++++++++++--------------------- cfg.mk | 53 ++++++------- 2 files changed, 193 insertions(+), 113 deletions(-) diff --git a/autogen.sh b/autogen.sh index d1c319d..d5d836a 100755 --- a/autogen.sh +++ b/autogen.sh @@ -1,117 +1,196 @@ #!/bin/sh # Run this to generate all the initial makefiles, etc. -set -e +die() +{ + echo "error: $1" >&2 + exit 1 +} -srcdir=`dirname "$0"` -test -z "$srcdir" && srcdir=. +starting_point=$(pwd) -THEDIR=`pwd` -cd "$srcdir" +srcdir=$(dirname "$0") +test "$srcdir" || srcdir=. -test -f src/libvirt.c || { - echo "You must run this script in the top-level libvirt directory" - exit 1 +cd "$srcdir" || { + die "Failed to cd into $srcdir" } +test -f src/libvirt.c || { + die "$0 must live in the top-level libvirt directory" +} -EXTRA_ARGS= +dry_run= no_git= -if test "x$1" = "x--no-git"; then - no_git=" $1" - shift - case "$1 $2" in - --gnulib-srcdir=*) no_git="$no_git $1"; shift ;; - --gnulib-srcdir\ *) no_git="$no_git $1=$2"; shift; shift;; - esac -fi -if test -z "$NOCONFIGURE" ; then - if test "x$1" = "x--system"; then - shift - prefix=/usr - libdir=$prefix/lib - sysconfdir=/etc - localstatedir=/var - if [ -d /usr/lib64 ]; then - libdir=$prefix/lib64 - fi - EXTRA_ARGS="--prefix=$prefix --sysconfdir=$sysconfdir --localstatedir=$localstatedir --libdir=$libdir" - echo "Running ./configure with $EXTRA_ARGS $@" - else - if test -z "$*" && test ! -f "$THEDIR/config.status"; then - echo "I am going to run ./configure with no arguments - if you wish" - echo "to pass any to it, please specify them on the $0 command line." - fi - fi -fi +gnulib_srcdir= +extra_args= +while test "$#" -gt 0; do + case "$1" in + --dry-run) + # This variable will serve both as an indicator of the fact that + # a dry run has been requested, and to store the result of the + # dry run. It will be ultimately used as return code for the + # script: 0 means no action is necessary, 2 means that autogen.sh + # needs to be executed, and 1 is reserved for failures + dry_run=0 + shift + ;; + --no-git) + no_git=" $1" + shift + ;; + --gnulib-srcdir=*) + gnulib_srcdir=" $1" + shift + ;; + --gnulib-srcdir) + gnulib_srcdir=" $1=$2" + shift + shift + ;; + --system) + prefix=/usr + sysconfdir=/etc + localstatedir=/var + if test -d $prefix/lib64; then + libdir=$prefix/lib64 + else + libdir=$prefix/lib + fi + extra_args="--prefix=$prefix --localstatedir=$localstatedir" + extra_args="$extra_args --sysconfdir=$sysconfdir --libdir=$libdir" + shift + ;; + *) + # All remaining arguments will be passed to configure verbatim + break + ;; + esac +done +no_git="$no_git$gnulib_srcdir" -# Compute the hash we'll use to determine whether rerunning bootstrap -# is required. The first is just the SHA1 that selects a gnulib snapshot. -# The second ensures that whenever we change the set of gnulib modules used -# by this package, we rerun bootstrap to pull in the matching set of files. -# The third ensures that whenever we change the set of local gnulib diffs, -# we rerun bootstrap to pull in those diffs. -bootstrap_hash() +gnulib_hash() { + local no_git=$1 + if test "$no_git"; then - echo no-git + echo "no-git" return fi - git submodule status | sed 's/^[ +-]//;s/ .*//' + + # Compute the hash we'll use to determine whether rerunning bootstrap + # is required. The first is just the SHA1 that selects a gnulib snapshot. + # The second ensures that whenever we change the set of gnulib modules used + # by this package, we rerun bootstrap to pull in the matching set of files. + # The third ensures that whenever we change the set of local gnulib diffs, + # we rerun bootstrap to pull in those diffs. + git submodule status .gnulib | awk '{ print $1 }' git hash-object bootstrap.conf - git ls-tree -d HEAD gnulib/local | awk '{print $3}' + git ls-tree -d HEAD gnulib/local | awk '{ print $3 }' } -# Ensure that whenever we pull in a gnulib update or otherwise change to a -# different version (i.e., when switching branches), we also rerun ./bootstrap. -# Also, running 'make rpm' tends to litter the po/ directory, and some people -# like to run 'git clean -x -f po' to fix it; but only ./bootstrap regenerates -# the required file po/Makevars. -# Only run bootstrap from a git checkout, never from a tarball. +# Only look into git submodules if we're in a git checkout if test -d .git || test -f .git; then - curr_status=.git-module-status t= - if test "$no_git"; then - t=no-git - elif test -d .gnulib; then - t=$(bootstrap_hash; git diff .gnulib) + + # Check for dirty submodules + if test -z "$CLEAN_SUBMODULE"; then + for path in $(git submodule status | awk '{ print $2 }'); do + case "$(git diff "$path")" in + *-dirty*) + echo "error: $path is dirty, please investigate" >&2 + echo "set CLEAN_SUBMODULE to discard submodule changes" >&2 + exit 1 + ;; + esac + done fi - case $t:${CLEAN_SUBMODULE+set} in - *:set) ;; - *-dirty*) - echo "error: gnulib submodule is dirty, please investigate" 2>&1 - echo "set env-var CLEAN_SUBMODULE to discard gnulib changes" 2>&1 - exit 1 ;; - esac - # Keep this test in sync with cfg.mk:_update_required - if test "$t" = "$(cat $curr_status 2>/dev/null)" \ - && test -f "po/Makevars" && test -f AUTHORS; then - # good, it's up to date, all we need is autoreconf - autoreconf -if + if test "$CLEAN_SUBMODULE" && test -z "$no_git"; then + if test -z "$dry_run"; then + echo "Cleaning up submodules..." + git submodule foreach 'git clean -dfqx && git reset --hard' || { + die "Cleaning up submodules failed" + } + fi + fi + + # Update all submodules. If any of the submodules has not been + # initialized yet, it will be initialized now; moreover, any submodule + # with uncommitted changes will be returned to the expected state + echo "Updating submodules..." + git submodule update --init || { + die "Updating submodules failed" + } + + # The expected hash, eg. the one computed after the last + # successful bootstrap run, is stored on disk + state_file=.git-module-status + expected_hash=$(cat "$state_file" 2>/dev/null) + actual_hash=$(gnulib_hash "$no_git") + + if test "$actual_hash" = "$expected_hash" && \ + test -f po/Makevars && test -f AUTHORS; then + # The gnulib hash matches our expectations, and all the files + # that can only be generated through bootstrap are present: + # we just need to run autoreconf. Unless we're performing a + # dry run, of course... + if test -z "$dry_run"; then + echo "Running autoreconf..." + autoreconf -if || { + die "autoreconf failed" + } + fi else - if test -z "$no_git" && test ${CLEAN_SUBMODULE+set}; then - echo cleaning up submodules... - git submodule foreach 'git clean -dfqx && git reset --hard' + # Whenever the gnulib submodule or any of the related bits + # has been changed in some way (see gnulib_hash) we need to + # run bootstrap again. If we're performing a dry run, we + # change the return code instead to signal our caller + if test "$dry_run"; then + dry_run=2 + else + echo "Running bootstrap..." + ./bootstrap$no_git --bootstrap-sync || { + die "bootstrap failed" + } + gnulib_hash >"$state_file" fi - echo running bootstrap$no_git... - ./bootstrap$no_git --bootstrap-sync && bootstrap_hash > $curr_status \ - || { echo "Failed to bootstrap, please investigate."; exit 1; } fi fi -test -n "$NOCONFIGURE" && exit 0 +# When performing a dry run, we can stop here +test "$dry_run" && exit "$dry_run" -cd "$THEDIR" +# If asked not to run configure, we can stop here +test "$NOCONFIGURE" && exit 0 -if test "x$OBJ_DIR" != x; then - mkdir -p "$OBJ_DIR" - cd "$OBJ_DIR" +cd "$starting_point" || { + die "Failed to cd into $starting_point" +} + +if test "$OBJ_DIR"; then + mkdir -p "$OBJ_DIR" || { + die "Failed to create $OBJ_DIR" + } + cd "$OBJ_DIR" || { + die "Failed to cd into $OBJ_DIR" + } fi -if test -z "$*" && test -z "$EXTRA_ARGS" && test -f config.status; then - ./config.status --recheck +if test -z "$*" && test -z "$extra_args" && test -f config.status; then + echo "Running config.status..." + ./config.status --recheck || { + die "config.status failed" + } else - $srcdir/configure $EXTRA_ARGS "$@" -fi && { - echo - echo "Now type 'make' to compile libvirt." -} + if test -z "$*" && test -z "$extra_args"; then + echo "I am going to run configure with no arguments - if you wish" + echo "to pass any to it, please specify them on the $0 command line." + else + echo "Running configure with $extra_args $@" + fi + "$srcdir/configure" $extra_args "$@" || { + die "configure failed" + } +fi + +echo +echo "Now type 'make' to compile libvirt." diff --git a/cfg.mk b/cfg.mk index 09548fe..c04a991 100644 --- a/cfg.mk +++ b/cfg.mk @@ -1026,33 +1026,30 @@ prev_version_file = /dev/null ifneq ($(_gl-Makefile),) ifeq (0,$(MAKELEVEL)) - _curr_status = .git-module-status - # The sed filter accommodates those who check out on a commit from which - # no tag is reachable. In that case, git submodule status prints a "-" - # in column 1 and does not print a "git describe"-style string after the - # submodule name. Contrast these: - # -b653eda3ac4864de205419d9f41eec267cb89eeb .gnulib - # b653eda3ac4864de205419d9f41eec267cb89eeb .gnulib (v0.0-2286-gb653eda) - # $ cat .git-module-status - # b653eda3ac4864de205419d9f41eec267cb89eeb - # - # Keep this logic in sync with autogen.sh. - _submodule_hash = $(SED) 's/^[ +-]//;s/ .*//' - _update_required := $(shell \ - cd '$(srcdir)'; \ - test -d .git || { echo 0; exit; }; \ - test -f po/Makevars || { echo 1; exit; }; \ - test -f AUTHORS || { echo 1; exit; }; \ - test "no-git" = "$$(cat $(_curr_status))" && { echo 0; exit; }; \ - actual=$$(git submodule status | $(_submodule_hash); \ - git hash-object bootstrap.conf; \ - git ls-tree -d HEAD gnulib/local | awk '{print $$3}'; \ - git diff .gnulib); \ - stamp="$$($(_submodule_hash) $(_curr_status) 2>/dev/null)"; \ - test "$$stamp" = "$$actual"; echo $$?) + _dry_run_result := $(shell \ + cd '$(srcdir)'; \ + test -d .git || test -f .git || { echo 0; exit; }; \ + $(srcdir)/autogen.sh --dry-run >/dev/null 2>&1; \ + echo $$?; \ + ) _clean_requested = $(filter %clean,$(MAKECMDGOALS)) - ifeq (1,$(_update_required)$(_clean_requested)) - $(info INFO: gnulib update required; running ./autogen.sh first) + + # A return value of 0 means no action is required + + # A return value of 1 means a genuine error has occurred while + # performing the dry run, and it should be reported so it can + # be investigated + ifeq (1,$(_dry_run_result)) + $(info INFO: autogen.sh error, running again to show details) +maint.mk Makefile: _autogen_error + endif + + # A return value of 2 means that autogen.sh needs to be executed + # in earnest before building, probably because of gnulib updates. + # We don't run autogen.sh if the clean target has been invoked, + # though, as it would be quite pointless + ifeq (2,$(_dry_run_result)$(_clean_requested)) + $(info INFO: running autogen.sh is required, running it now...) $(shell touch $(srcdir)/AUTHORS $(srcdir)/ChangeLog) maint.mk Makefile: _autogen endif @@ -1066,6 +1063,10 @@ _autogen: $(srcdir)/autogen.sh ./config.status +.PHONY: _autogen_error +_autogen_error: + $(srcdir)/autogen.sh --dry-run + # regenerate HACKING as part of the syntax-check ifneq ($(_gl-Makefile),) syntax-check: $(top_srcdir)/HACKING spacing-check test-wrap-argv \ -- 2.7.4

2 2

[libvirt] [PATCH] test: Remove unnecessary unlocks in cleanup paths
by John Ferlan 25 Apr '17

25 Apr '17

Commit id '865f479da' altered the logic to use a common test*ObjFindByName helpers which would lock/unlock the test driver; however, a few cleanup paths in that cleanup missed removing the Unlock, so remove it now. Signed-off-by: John Ferlan <jferlan(a)redhat.com> --- src/test/test_driver.c | 5 ----- 1 file changed, 5 deletions(-) diff --git a/src/test/test_driver.c b/src/test/test_driver.c index 00999b1..2db3f7d 100644 --- a/src/test/test_driver.c +++ b/src/test/test_driver.c @@ -3931,7 +3931,6 @@ testInterfaceUndefine(virInterfacePtr iface) ret = 0; cleanup: - testDriverUnlock(privconn); return ret; } @@ -3960,7 +3959,6 @@ testInterfaceCreate(virInterfacePtr iface, cleanup: if (privinterface) virInterfaceObjUnlock(privinterface); - testDriverUnlock(privconn); return ret; } @@ -3989,7 +3987,6 @@ testInterfaceDestroy(virInterfacePtr iface, cleanup: if (privinterface) virInterfaceObjUnlock(privinterface); - testDriverUnlock(privconn); return ret; } @@ -4480,7 +4477,6 @@ testStoragePoolUndefine(virStoragePoolPtr pool) if (privpool) virStoragePoolObjUnlock(privpool); testObjectEventQueue(privconn, event); - testDriverUnlock(privconn); return ret; } @@ -4591,7 +4587,6 @@ testStoragePoolDestroy(virStoragePoolPtr pool) testObjectEventQueue(privconn, event); if (privpool) virStoragePoolObjUnlock(privpool); - testDriverUnlock(privconn); return ret; } -- 2.9.3

2 1