August 2015 - Devel - libvirt List Archives

[libvirt] [PATCH] virNetSocketCheckProtocols: handle EAI_NONAME as IPv6 unavailable
by Guido Günther 11 Aug '15

11 Aug '15

When running the test suite using "unshare -n" we might have IPv6 but no configured addresses. Due to AI_ADDRCONFIG getaddrinfo then fails with EAI_NONAME which we should then treat as IPv6 unavailable. --- src/rpc/virnetsocket.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/rpc/virnetsocket.c b/src/rpc/virnetsocket.c index 106d09a..5e5f1ab 100644 --- a/src/rpc/virnetsocket.c +++ b/src/rpc/virnetsocket.c @@ -183,7 +183,8 @@ int virNetSocketCheckProtocols(bool *hasIPv4, if ((gaierr = getaddrinfo("::1", NULL, &hints, &ai)) != 0) { if (gaierr == EAI_ADDRFAMILY || - gaierr == EAI_FAMILY) { + gaierr == EAI_FAMILY || + gaierr == EAI_NONAME) { *hasIPv6 = false; } else { virReportError(VIR_ERR_INTERNAL_ERROR, -- 2.1.4

2 2

[libvirt] [PATCHv2] util: don't overwrite stack when getting ethtool gfeatures
by Laine Stump 11 Aug '15

11 Aug '15

This fixes the crash described here: https://www.redhat.com/archives/libvir-list/2015-August/msg00162.html In short, we were calling ioctl(SIOCETHTOOL) pointing to a too-short object that was a local on the stack, resulting in the memory past the end of the object being overwritten. This was because the struct used by the ETHTOOL_GFEATURES command of SIOCETHTOOL ends with a 0-length array, but we were telling ethtool that it could use 2 elements on the array. The fix is to allocate the necessary memory with VIR_ALLOC_VAR(), including the extra length needed for a 2 element array at the end. --- V2 difference: eliminate all the other cleanups that were in V1, just fix the crash. I had misunderstood that a "failure" of the ioctl shouldn't really be considered a *failure*, and don't have time to tweak around with all the inconsequential stuff right now. src/util/virnetdev.c | 12 ++++++++---- 1 file changed, 8 insertions(+), 4 deletions(-) diff --git a/src/util/virnetdev.c b/src/util/virnetdev.c index 1e20789..c158ca3 100644 --- a/src/util/virnetdev.c +++ b/src/util/virnetdev.c @@ -3130,7 +3130,7 @@ virNetDevGetFeatures(const char *ifname, size_t i = -1; struct ethtool_value cmd = { 0 }; # if HAVE_DECL_ETHTOOL_GFEATURES - struct ethtool_gfeatures g_cmd = { 0 }; + struct ethtool_gfeatures *g_cmd; # endif struct elem{ const int cmd; @@ -3188,10 +3188,14 @@ virNetDevGetFeatures(const char *ifname, # endif # if HAVE_DECL_ETHTOOL_GFEATURES - g_cmd.cmd = ETHTOOL_GFEATURES; - g_cmd.size = GFEATURES_SIZE; - if (virNetDevGFeatureAvailable(ifname, &g_cmd)) + if (VIR_ALLOC_VAR(g_cmd, + struct ethtool_get_features_block, GFEATURES_SIZE) < 0) + return -1; + g_cmd->cmd = ETHTOOL_GFEATURES; + g_cmd->size = GFEATURES_SIZE; + if (virNetDevGFeatureAvailable(ifname, g_cmd)) ignore_value(virBitmapSetBit(*out, VIR_NET_DEV_FEAT_TXUDPTNL)); + VIR_FREE(g_cmd); # endif if (virNetDevRDMAFeature(ifname, out) < 0) -- 2.1.0

2 1

[libvirt] [PATCH] util: fix virNetDevSendEthtoolIoctl() and its callers
by Laine Stump 11 Aug '15

11 Aug '15

This started out as a fix for a crash reported in IRC and on libvir-list: https://www.redhat.com/archives/libvir-list/2015-August/msg00162.html but as I examined the existing code I found so many nits to pick that I just did them all. The most important fix here is that virNetDevGetFeatures was creating a struct ethtool_gfeatures object as a local on the stack and, although the struct is defined with 0 elements in its features array, we were telling the ethtool ioctl that we had made space for 2 elements. This led to a crash, as outlined in the email above. The fix for this is to allocate the memory for the ethtool_gfeatures object using VIR_ALLOC_N to a char*, giving it the length: sizeof(ethtool_gfeatures) + (2 * sizeof(ethtool_get_features_block) because VIR_ALLOC_N is a macro and fails when you try to typecast a pointer to char* within the invocation, I made a union that has both a char* and an ethtool_gfeatures*, and used the char* of the union for VIR_ALLOC_N, and the other for everything else. Beyond that crash fixer, the following fixups were made to the hierarchy of functions between virNetDevGetFeatures() and virNetDevSendEthtoolIoctl(): * macros used to examine the gfeatures bits were renamed from FEATURE_* to GFEATURE_* virNetDevSentEthtoolIoctl(): * no need to initialize sock to -1, since it is always set at the top of the function. * remove VIR_DEBUG log (and subsequent *skipping* of error log!) when errno is EPERM, EINVAL or EOPNOTSUPP. If one of those errors were ever encountered, this would have been *very* problematic, as it would have led to one of those situations where virsh reports "an error was encountered but the cause is unknown" (or whatever the message is when we have an error but no log message). * always call VIR_FORCE_CLOSE(sock) since we know that sock is either a valid fd, or else -1 (which VIR_FORCE_CLOSE() will skip). virNetDevFeatureAvailable() * simplify it - no need for ret. * follow libvirt convention of checking for "bobLobLaw(lawblog) < 0" instead of "!bobLobLaw(lawblog)". virNetDevGFeatureAvailable() * eliminate this function, as it was ill-conceived (it really was only checking for one gfeature (TX_UDP_TNL), not *any* gfeature. virNetDevGetFeatures() * move all data tables (struct elem) out of the function so that they will be statics instead of taking up space on the stack. * remove pointless/incorrect initialization of i = -1. * remove unnecessary static initialization of struct ethtool_value cmd * put struct ethtool_gfeatures into a union with a char* as described above * use libvirt convention of checking return from virNetDevFeatureAvailable() < 0, instead of "!virNetDevFeatureAvailable()", and immediately return to caller with an error when virNetDevFeatureAvailable() returns an error (previously it was ignored). * remove superfluous size_t j, and just re-use i instead. * runtime allocation/free of proper size object for ethtoolfeatures as described above. * directly call virNetDevSentEthtoolIoctl() instead of now defunct virNetDevGFeatureAvailable(). === NB: This patch does *not* attempt to determine the proper number of elements for the gfeature array at runtime, as proposed in this patch: https://www.redhat.com/archives/libvir-list/2015-August/msg00263.html since that wasn't the cause of the crash. I'll leave it up to Moshe to repost that patch rebased around this one (or whatever ends up being pushed) if he thinks there is value to it. Also - as I mentioned in earlier mail in response to the crash, I noticed when looking into the gfeatures ethtool code that it looks to me like TX_UDP_TNL should actually be 26 rather than 25, but I may be missing something. Moshe - can you either confirm or deny that? Where did you get the value 25 from? --- src/util/virnetdev.c | 182 ++++++++++++++++++++++++--------------------------- 1 file changed, 84 insertions(+), 98 deletions(-) diff --git a/src/util/virnetdev.c b/src/util/virnetdev.c index 1e20789..7f0837d 100644 --- a/src/util/virnetdev.c +++ b/src/util/virnetdev.c @@ -91,10 +91,10 @@ VIR_LOG_INIT("util.netdev"); #if HAVE_DECL_ETHTOOL_GFEATURES # define TX_UDP_TNL 25 # define GFEATURES_SIZE 2 -# define FEATURE_WORD(blocks, index, field) ((blocks)[(index) / 32U].field) -# define FEATURE_FIELD_FLAG(index) (1U << (index) % 32U) -# define FEATURE_BIT_IS_SET(blocks, index, field) \ - (FEATURE_WORD(blocks, index, field) & FEATURE_FIELD_FLAG(index)) +# define GFEATURE_WORD(blocks, index, field) ((blocks)[(index) / 32U].field) +# define GFEATURE_FIELD_FLAG(index) (1U << (index) % 32U) +# define GFEATURE_BIT_IS_SET(blocks, index, field) \ + (GFEATURE_WORD(blocks, index, field) & GFEATURE_FIELD_FLAG(index)) #endif typedef enum { @@ -3032,11 +3032,10 @@ static int virNetDevSendEthtoolIoctl(const char *ifname, void *cmd) { int ret = -1; - int sock = -1; + int sock; virIfreq ifr; - sock = socket(AF_LOCAL, SOCK_DGRAM, 0); - if (sock < 0) { + if ((sock = socket(AF_LOCAL, SOCK_DGRAM, 0)) < 0) { virReportSystemError(errno, "%s", _("Cannot open control socket")); goto cleanup; } @@ -3044,27 +3043,11 @@ virNetDevSendEthtoolIoctl(const char *ifname, void *cmd) memset(&ifr, 0, sizeof(ifr)); strcpy(ifr.ifr_name, ifname); ifr.ifr_data = cmd; - ret = ioctl(sock, SIOCETHTOOL, &ifr); - if (ret != 0) { - switch (errno) { - case EPERM: - VIR_DEBUG("ethtool ioctl: permission denied"); - break; - case EINVAL: - VIR_DEBUG("ethtool ioctl: invalid request"); - break; - case EOPNOTSUPP: - VIR_DEBUG("ethtool ioctl: request not supported"); - break; - default: - virReportSystemError(errno, "%s", _("ethtool ioctl error")); - goto cleanup; - } - } + if ((ret = ioctl(sock, SIOCETHTOOL, &ifr)) < 0) + virReportSystemError(errno, "%s", _("ethtool ioctl error")); cleanup: - if (sock) - VIR_FORCE_CLOSE(sock); + VIR_FORCE_CLOSE(sock); return ret; } @@ -3081,35 +3064,50 @@ virNetDevSendEthtoolIoctl(const char *ifname, void *cmd) static int virNetDevFeatureAvailable(const char *ifname, struct ethtool_value *cmd) { - int ret = -1; - cmd = (void*)cmd; - if (!virNetDevSendEthtoolIoctl(ifname, cmd)) - ret = cmd->data > 0 ? 1 : 0; - return ret; + if (virNetDevSendEthtoolIoctl(ifname, cmd) < 0) + return -1; + return cmd->data > 0 ? 1 : 0; } -# if HAVE_DECL_ETHTOOL_GFEATURES -/** - * virNetDevGFeatureAvailable - * This function checks for the availability of a network device gfeature - * - * @ifname: name of the interface - * @cmd: reference to a gfeatures ethtool command structure - * - * Returns 0 if not found, 1 on success, and -1 on failure. - */ -static int -virNetDevGFeatureAvailable(const char *ifname, struct ethtool_gfeatures *cmd) -{ - int ret = -1; +/* static tables used by virNetDevGetFeatures() */ +struct elem { + const int cmd; + const virNetDevFeature feat; +}; - cmd = (void*)cmd; - if (!virNetDevSendEthtoolIoctl(ifname, cmd)) - ret = FEATURE_BIT_IS_SET(cmd->features, TX_UDP_TNL, active); - return ret; -} +/* legacy ethtool getters */ +struct elem cmds[] = { + {ETHTOOL_GRXCSUM, VIR_NET_DEV_FEAT_GRXCSUM}, + {ETHTOOL_GTXCSUM, VIR_NET_DEV_FEAT_GTXCSUM}, + {ETHTOOL_GSG, VIR_NET_DEV_FEAT_GSG}, + {ETHTOOL_GTSO, VIR_NET_DEV_FEAT_GTSO}, +# if HAVE_DECL_ETHTOOL_GGSO + {ETHTOOL_GGSO, VIR_NET_DEV_FEAT_GGSO}, +# endif +# if HAVE_DECL_ETHTOOL_GGRO + {ETHTOOL_GGRO, VIR_NET_DEV_FEAT_GGRO}, +# endif +}; + +# if HAVE_DECL_ETHTOOL_GFLAGS +/* ethtool masks */ +struct elem flags[] = { +# if HAVE_DECL_ETH_FLAG_LRO + {ETH_FLAG_LRO, VIR_NET_DEV_FEAT_LRO}, +# endif +# if HAVE_DECL_ETH_FLAG_TXVLAN + {ETH_FLAG_RXVLAN, VIR_NET_DEV_FEAT_RXVLAN}, + {ETH_FLAG_TXVLAN, VIR_NET_DEV_FEAT_TXVLAN}, +# endif +# if HAVE_DECL_ETH_FLAG_NTUBLE + {ETH_FLAG_NTUPLE, VIR_NET_DEV_FEAT_NTUPLE}, +# endif +# if HAVE_DECL_ETH_FLAG_RXHASH + {ETH_FLAG_RXHASH, VIR_NET_DEV_FEAT_RXHASH}, +# endif +}; # endif @@ -3127,71 +3125,59 @@ int virNetDevGetFeatures(const char *ifname, virBitmapPtr *out) { - size_t i = -1; - struct ethtool_value cmd = { 0 }; + size_t i; + int ret; + struct ethtool_value cmd; # if HAVE_DECL_ETHTOOL_GFEATURES - struct ethtool_gfeatures g_cmd = { 0 }; + union { + struct ethtool_gfeatures *cmd; + char *cmd_char; + } g; # endif - struct elem{ - const int cmd; - const virNetDevFeature feat; - }; - /* legacy ethtool getters */ - struct elem cmds[] = { - {ETHTOOL_GRXCSUM, VIR_NET_DEV_FEAT_GRXCSUM}, - {ETHTOOL_GTXCSUM, VIR_NET_DEV_FEAT_GTXCSUM}, - {ETHTOOL_GSG, VIR_NET_DEV_FEAT_GSG}, - {ETHTOOL_GTSO, VIR_NET_DEV_FEAT_GTSO}, -# if HAVE_DECL_ETHTOOL_GGSO - {ETHTOOL_GGSO, VIR_NET_DEV_FEAT_GGSO}, -# endif -# if HAVE_DECL_ETHTOOL_GGRO - {ETHTOOL_GGRO, VIR_NET_DEV_FEAT_GGRO}, -# endif - }; if (!(*out = virBitmapNew(VIR_NET_DEV_FEAT_LAST))) return -1; + /* first set of capabilities are one capability per + * command. cmd.data is set if the interface has that + * capability + */ for (i = 0; i < ARRAY_CARDINALITY(cmds); i++) { cmd.cmd = cmds[i].cmd; - if (virNetDevFeatureAvailable(ifname, &cmd)) + if ((ret = virNetDevFeatureAvailable(ifname, &cmd)) < 0) + return -1; + if (ret) ignore_value(virBitmapSetBit(*out, cmds[i].feat)); } # if HAVE_DECL_ETHTOOL_GFLAGS - size_t j = -1; - /* ethtool masks */ - struct elem flags[] = { -# if HAVE_DECL_ETH_FLAG_LRO - {ETH_FLAG_LRO, VIR_NET_DEV_FEAT_LRO}, -# endif -# if HAVE_DECL_ETH_FLAG_TXVLAN - {ETH_FLAG_RXVLAN, VIR_NET_DEV_FEAT_RXVLAN}, - {ETH_FLAG_TXVLAN, VIR_NET_DEV_FEAT_TXVLAN}, -# endif -# if HAVE_DECL_ETH_FLAG_NTUBLE - {ETH_FLAG_NTUPLE, VIR_NET_DEV_FEAT_NTUPLE}, -# endif -# if HAVE_DECL_ETH_FLAG_RXHASH - {ETH_FLAG_RXHASH, VIR_NET_DEV_FEAT_RXHASH}, -# endif - }; - + /* second set of capabilities are all stored as 1 bit each in + * cmd.data of the result of the single ETHTOOL_GFLAGS command + */ cmd.cmd = ETHTOOL_GFLAGS; - if (virNetDevFeatureAvailable(ifname, &cmd)) { - for (j = 0; j < ARRAY_CARDINALITY(flags); j++) { - if (cmd.data & flags[j].cmd) - ignore_value(virBitmapSetBit(*out, flags[j].feat)); - } - } + if ((ret = virNetDevFeatureAvailable(ifname, &cmd)) < 0) + return -1; + for (i = 0; ret && i < ARRAY_CARDINALITY(flags); i++) + if (cmd.data & flags[i].cmd) + ignore_value(virBitmapSetBit(*out, flags[i].feat)); # endif # if HAVE_DECL_ETHTOOL_GFEATURES - g_cmd.cmd = ETHTOOL_GFEATURES; - g_cmd.size = GFEATURES_SIZE; - if (virNetDevGFeatureAvailable(ifname, &g_cmd)) + /* allocate an object with GFEATURES_SIZE elements in the features array */ + if (VIR_ALLOC_N(g.cmd_char, + sizeof(struct ethtool_gfeatures) + + (sizeof(struct ethtool_get_features_block) * GFEATURES_SIZE)) < 0) + return -1; + + g.cmd->cmd = ETHTOOL_GFEATURES; + g.cmd->size = GFEATURES_SIZE; + if (virNetDevSendEthtoolIoctl(ifname, g.cmd) < 0) { + VIR_FREE(g.cmd); + return -1; + } + if (GFEATURE_BIT_IS_SET(g.cmd->features, TX_UDP_TNL, active)) ignore_value(virBitmapSetBit(*out, VIR_NET_DEV_FEAT_TXUDPTNL)); + VIR_FREE(g.cmd); # endif if (virNetDevRDMAFeature(ifname, out) < 0) -- 2.1.0

3 5

[libvirt] [PATCH] cpu: Fix segfault in the ppc64 driver
by Andrea Bolognani 11 Aug '15

11 Aug '15

Commit adb865d introduced some changes in ppc64DriverNodeData() that cause libvirtd to crash on startup unless this patch is applied as well. --- src/cpu/cpu_ppc64.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/cpu/cpu_ppc64.c b/src/cpu/cpu_ppc64.c index 33fec8b..85aa5bc 100644 --- a/src/cpu/cpu_ppc64.c +++ b/src/cpu/cpu_ppc64.c @@ -628,7 +628,7 @@ ppc64DriverNodeData(virArch arch) if (VIR_ALLOC(nodeData) < 0) goto error; - if (VIR_ALLOC(data) < 0) + if (VIR_ALLOC(nodeData->data.ppc64) < 0) goto error; data = nodeData->data.ppc64; -- 2.4.3

2 2

[libvirt] [PATCH] conf: Don't try formating non-existing addresses
by Martin Kletzander 11 Aug '15

11 Aug '15

Commit a6f9af8292b6 added checking for address colisions between starting and ending addresses of forwarding addresses, but forgot that there might be no addresses set at all. Signed-off-by: Martin Kletzander <mkletzan(a)redhat.com> --- src/conf/network_conf.c | 22 +++++++++++++++++++--- ....xml => nat-network-forward-nat-no-address.xml} | 1 - ....xml => nat-network-forward-nat-no-address.xml} | 1 - tests/networkxml2xmltest.c | 1 + 4 files changed, 20 insertions(+), 5 deletions(-) copy tests/networkxml2xmlin/{nat-network-forward-nat-address.xml => nat-network-forward-nat-no-address.xml} (93%) copy tests/networkxml2xmlout/{nat-network-forward-nat-address.xml => nat-network-forward-nat-no-address.xml} (93%) diff --git a/src/conf/network_conf.c b/src/conf/network_conf.c index 374d723788e1..f9d894b12046 100644 --- a/src/conf/network_conf.c +++ b/src/conf/network_conf.c @@ -1731,9 +1731,25 @@ virNetworkForwardNatDefParseXML(const char *networkName, goto cleanup; } - /* verify that start <= end */ - if (virSocketAddrGetRange(&def->addr.start, &def->addr.end, NULL, 0) < 0) - goto cleanup; + if (addrStart && addrEnd) { + /* verify that start <= end */ + if (virSocketAddrGetRange(&def->addr.start, &def->addr.end, NULL, 0) < 0) + goto cleanup; + } else { + if (addrStart) { + virReportError(VIR_ERR_XML_ERROR, + _("Only start address '%s' specified in <nat> in " + "<forward> in network '%s'"), + addrStart, networkName); + goto cleanup; + } + if (addrEnd) { + virReportError(VIR_ERR_XML_ERROR, + _("Only end address '%s' specified in <nat> in " + "<forward> in network '%s'"), + addrEnd, networkName); + } + } /* ports for SNAT and MASQUERADE */ nNatPorts = virXPathNodeSet("./port", ctxt, &natPortNodes); diff --git a/tests/networkxml2xmlin/nat-network-forward-nat-address.xml b/tests/networkxml2xmlin/nat-network-forward-nat-no-address.xml similarity index 93% copy from tests/networkxml2xmlin/nat-network-forward-nat-address.xml copy to tests/networkxml2xmlin/nat-network-forward-nat-no-address.xml index 403d058330ea..97a64526e94f 100644 --- a/tests/networkxml2xmlin/nat-network-forward-nat-address.xml +++ b/tests/networkxml2xmlin/nat-network-forward-nat-no-address.xml @@ -4,7 +4,6 @@ <bridge name="virbr0"/> <forward mode="nat" dev="eth1"> <nat> - <address start='10.20.30.40' end='10.20.30.44'/> <port start='60000' end='65432'/> </nat> </forward> diff --git a/tests/networkxml2xmlout/nat-network-forward-nat-address.xml b/tests/networkxml2xmlout/nat-network-forward-nat-no-address.xml similarity index 93% copy from tests/networkxml2xmlout/nat-network-forward-nat-address.xml copy to tests/networkxml2xmlout/nat-network-forward-nat-no-address.xml index faeba2470661..f19e34deecec 100644 --- a/tests/networkxml2xmlout/nat-network-forward-nat-address.xml +++ b/tests/networkxml2xmlout/nat-network-forward-nat-no-address.xml @@ -3,7 +3,6 @@ <uuid>81ff0d90-c91e-6742-64da-4a736edb9a9b</uuid> <forward dev='eth1' mode='nat'> <nat> - <address start='10.20.30.40' end='10.20.30.44'/> <port start='60000' end='65432'/> </nat> <interface dev='eth1'/> diff --git a/tests/networkxml2xmltest.c b/tests/networkxml2xmltest.c index 290336edbefd..8d60aa8fc665 100644 --- a/tests/networkxml2xmltest.c +++ b/tests/networkxml2xmltest.c @@ -99,6 +99,7 @@ mymain(void) DO_TEST("nat-network-dns-forward-plain"); DO_TEST("nat-network-dns-forwarders"); DO_TEST("nat-network-forward-nat-address"); + DO_TEST("nat-network-forward-nat-no-address"); DO_TEST("8021Qbh-net"); DO_TEST("direct-net"); DO_TEST("host-bridge-net"); -- 2.5.0

3 3

[libvirt] [PATCH v3 00/20] cpu: Fix and improve the ppc64 driver
by Andrea Bolognani 11 Aug '15

11 Aug '15

Patches 01-07 are cleanups, 08-10 are bug fixes and 11-20 improve the driver. Changes in v3: * Fix a bug spotted by John thanks to Coverity Changes in v2: * Implement compatibility with guests defined on older libvirt versions * Always use fallback='forbid' when emitting CPU definitions * Update all tests in one go instead of changing them several times * Handle test failure and system failure differently in cpuTestGuestData() * Simplify ppc64DriverNodeData() * Simplify XML parsing code * Remove unnecessary NULL checks * Add even more test cases * Other small changes such as shuffling chunks between commits to make the history more readable Cheers. Andrea Bolognani (20): cpu: Mark driver functions in ppc64 driver cpu: Simplify NULL handling in ppc64 driver cpu: Simplify ppc64ModelFromCPU() cpu: Reorder functions in the ppc64 driver cpu: Remove ISA information from CPU map XML tests: Remove unused file tests: Improve result handling in cpuTestGuestData() cpu: Never skip CPU model name check in ppc64 driver cpu: CPU model names have to match on ppc64 cpu: Use ppc64Compute() to implement ppc64DriverCompare() tests: Temporarily disable ppc64 cpu tests cpu: Align ppc64 CPU data with x86 cpu: Support multiple PVRs in the ppc64 driver cpu: Simplify ppc64 part of CPU map XML cpu: Parse and use PVR masks in the ppc64 driver cpu: Add POWER8NVL information to CPU map XML cpu: Implement backwards compatibility in the ppc64 driver cpu: Forbid model fallback in the ppc64 driver tests: Re-enable ppc64 cpu tests tests: Add a bunch of cpu test case for ppc64 src/cpu/cpu.h | 2 +- src/cpu/cpu_map.xml | 59 +-- src/cpu/cpu_ppc64.c | 451 +++++++++++++-------- src/cpu/cpu_ppc64_data.h | 12 +- tests/cputest.c | 55 ++- tests/cputestdata/ppc64-baseline-1-result.xml | 3 - .../ppc64-baseline-incompatible-models.xml | 14 + .../ppc64-baseline-incompatible-vendors.xml | 4 +- tests/cputestdata/ppc64-baseline-legacy.xml | 14 + .../ppc64-baseline-no-vendor-result.xml | 2 +- tests/cputestdata/ppc64-baseline-no-vendor.xml | 2 +- .../ppc64-baseline-same-model-result.xml | 3 + tests/cputestdata/ppc64-baseline-same-model.xml | 14 + tests/cputestdata/ppc64-exact.xml | 3 - tests/cputestdata/ppc64-guest-exact.xml | 3 + .../ppc64-guest-legacy-incompatible.xml | 3 + tests/cputestdata/ppc64-guest-legacy-invalid.xml | 3 + tests/cputestdata/ppc64-guest-legacy.xml | 3 + tests/cputestdata/ppc64-guest-nofallback.xml | 3 +- tests/cputestdata/ppc64-guest-strict.xml | 3 + tests/cputestdata/ppc64-guest.xml | 3 +- .../ppc64-host+guest,ppc_models-result.xml | 2 +- ... ppc64-host+guest-legacy,ppc_models-result.xml} | 2 +- tests/cputestdata/ppc64-host-better.xml | 6 + tests/cputestdata/ppc64-host-incomp-arch.xml | 6 + tests/cputestdata/ppc64-host-no-vendor.xml | 5 + tests/cputestdata/ppc64-host-worse.xml | 6 + tests/cputestdata/ppc64-host.xml | 2 +- tests/cputestdata/ppc64-strict.xml | 3 - 29 files changed, 444 insertions(+), 247 deletions(-) delete mode 100644 tests/cputestdata/ppc64-baseline-1-result.xml create mode 100644 tests/cputestdata/ppc64-baseline-incompatible-models.xml create mode 100644 tests/cputestdata/ppc64-baseline-legacy.xml create mode 100644 tests/cputestdata/ppc64-baseline-same-model-result.xml create mode 100644 tests/cputestdata/ppc64-baseline-same-model.xml delete mode 100644 tests/cputestdata/ppc64-exact.xml create mode 100644 tests/cputestdata/ppc64-guest-exact.xml create mode 100644 tests/cputestdata/ppc64-guest-legacy-incompatible.xml create mode 100644 tests/cputestdata/ppc64-guest-legacy-invalid.xml create mode 100644 tests/cputestdata/ppc64-guest-legacy.xml create mode 100644 tests/cputestdata/ppc64-guest-strict.xml rename tests/cputestdata/{ppc64-host+guest-nofallback,ppc_models,POWER7_v2.1-result.xml => ppc64-host+guest-legacy,ppc_models-result.xml} (64%) create mode 100644 tests/cputestdata/ppc64-host-better.xml create mode 100644 tests/cputestdata/ppc64-host-incomp-arch.xml create mode 100644 tests/cputestdata/ppc64-host-no-vendor.xml create mode 100644 tests/cputestdata/ppc64-host-worse.xml delete mode 100644 tests/cputestdata/ppc64-strict.xml -- 2.4.3

2 43

[libvirt] [PATCH] network: validate network NAT range
by Laine Stump 11 Aug '15

11 Aug '15

This patch modifies virSocketAddrGetRange() to function properly when the containing network/prefix of the address range isn't known, for example in the case of the NAT range of a virtual network (since it is a range of addresses on the *host*, not within the network itself). We then take advantage of this new functionality to validate the NAT range of a virtual network. Extra test cases are also added to verify that virSocketAddrGetRange() works properly in both positive and negative cases when the network pointer is NULL. This is the *real* fix for: https://bugzilla.redhat.com/show_bug.cgi?id=985653 Commits 1e334a and 48e8b9 had earlier been pushed as fixes for that bug, but I had neglected to read the report carefully, so instead of fixing validation for the NAT range, I had fixed validation for the DHCP range. sigh. --- The changes to virSocketAddrGetRange() *look* like they are extensive, but really they almost completely consist of: 1) reordering and reindenting some of the checks so that they are only executed when we have a valid network address 2) modifying the error messages that could occur when there isn't a valid network so that they don't attempt to use the network address or prefix. src/conf/network_conf.c | 4 ++ src/util/virsocketaddr.c | 168 +++++++++++++++++++++++++---------------------- tests/sockettest.c | 46 ++++++++++++- 3 files changed, 136 insertions(+), 82 deletions(-) diff --git a/src/conf/network_conf.c b/src/conf/network_conf.c index f77af15..b8d918c 100644 --- a/src/conf/network_conf.c +++ b/src/conf/network_conf.c @@ -1731,6 +1731,10 @@ virNetworkForwardNatDefParseXML(const char *networkName, goto cleanup; } + /* verify that start <= end */ + if (virSocketAddrGetRange(&def->addr.start, &def->addr.end, NULL, 0) < 0) + goto cleanup; + /* ports for SNAT and MASQUERADE */ nNatPorts = virXPathNodeSet("./port", ctxt, &natPortNodes); if (nNatPorts < 0) { diff --git a/src/util/virsocketaddr.c b/src/util/virsocketaddr.c index 81539b3..900aa5b 100644 --- a/src/util/virsocketaddr.c +++ b/src/util/virsocketaddr.c @@ -628,126 +628,136 @@ virSocketAddrGetRange(virSocketAddrPtr start, virSocketAddrPtr end, virSocketAddr netmask; char *startStr = NULL, *endStr = NULL, *netStr = NULL; - if (start == NULL || end == NULL || network == NULL) { + if (start == NULL || end == NULL) { virReportError(VIR_ERR_INTERNAL_ERROR, - _("NULL argument - %p %p %p"), start, end, network); + _("NULL argument - %p %p"), start, end); goto error; } startStr = virSocketAddrFormat(start); endStr = virSocketAddrFormat(end); - netStr = virSocketAddrFormat(network); - if (!(startStr && endStr && netStr)) + if (!startStr || !endStr) goto error; /*error already reported */ - if (VIR_SOCKET_ADDR_FAMILY(start) != VIR_SOCKET_ADDR_FAMILY(end) || - VIR_SOCKET_ADDR_FAMILY(start) != VIR_SOCKET_ADDR_FAMILY(network)) { + if (VIR_SOCKET_ADDR_FAMILY(start) != VIR_SOCKET_ADDR_FAMILY(end)) { virReportError(VIR_ERR_INTERNAL_ERROR, - _("mismatch of address family in " - "range %s - %s for network %s"), - startStr, endStr, netStr); + _("mismatch of address family in range %s - %s"), + startStr, endStr); goto error; } - if (prefix < 0 || - virSocketAddrPrefixToNetmask(prefix, &netmask, - VIR_SOCKET_ADDR_FAMILY(network)) < 0) { - virReportError(VIR_ERR_INTERNAL_ERROR, - _("bad prefix %d for network %s when " - " checking range %s - %s"), - prefix, netStr, startStr, endStr); - goto error; - } - - /* both start and end of range need to be in the same network as - * "network" - */ - if (virSocketAddrCheckNetmask(start, network, &netmask) <= 0 || - virSocketAddrCheckNetmask(end, network, &netmask) <= 0) { - virReportError(VIR_ERR_INTERNAL_ERROR, - _("range %s - %s is not entirely within " - "network %s/%d"), - startStr, endStr, netStr, prefix); - goto error; - } - - if (VIR_SOCKET_ADDR_IS_FAMILY(start, AF_INET)) { - virSocketAddrIPv4 t1, t2; - virSocketAddr netaddr, broadcast; + if (network) { + /* some checks can only be done if we have details of the + * network the range should be within + */ + if (!(netStr = virSocketAddrFormat(network))) + goto error; - if (virSocketAddrBroadcast(network, &netmask, &broadcast) < 0 || - virSocketAddrMask(network, &netmask, &netaddr) < 0) { + if (VIR_SOCKET_ADDR_FAMILY(start) != VIR_SOCKET_ADDR_FAMILY(network)) { virReportError(VIR_ERR_INTERNAL_ERROR, - _("failed to construct broadcast or network " - "address for network %s/%d"), - netStr, prefix); + _("mismatch of address family in " + "range %s - %s for network %s"), + startStr, endStr, netStr); goto error; } - /* Don't allow the start of the range to be the network - * address (usually "...0") or the end of the range to be the - * broadcast address (usually "...255"). (the opposite also - * isn't allowed, but checking for that is implicit in all the - * other combined checks) (IPv6 doesn't have broadcast and - * network addresses, so this check is only done for IPv4) - */ - if (virSocketAddrEqual(start, &netaddr)) { + if (prefix < 0 || + virSocketAddrPrefixToNetmask(prefix, &netmask, + VIR_SOCKET_ADDR_FAMILY(network)) < 0) { virReportError(VIR_ERR_INTERNAL_ERROR, - _("start of range %s - %s in network %s/%d " - "is the network address"), - startStr, endStr, netStr, prefix); + _("bad prefix %d for network %s when " + " checking range %s - %s"), + prefix, netStr, startStr, endStr); goto error; } - if (virSocketAddrEqual(end, &broadcast)) { + /* both start and end of range need to be within network */ + if (virSocketAddrCheckNetmask(start, network, &netmask) <= 0 || + virSocketAddrCheckNetmask(end, network, &netmask) <= 0) { virReportError(VIR_ERR_INTERNAL_ERROR, - _("end of range %s - %s in network %s/%d " - "is the broadcast address"), + _("range %s - %s is not entirely within " + "network %s/%d"), startStr, endStr, netStr, prefix); goto error; } - if ((virSocketAddrGetIPv4Addr(start, &t1) < 0) || - (virSocketAddrGetIPv4Addr(end, &t2) < 0)) { + if (VIR_SOCKET_ADDR_IS_FAMILY(start, AF_INET)) { + virSocketAddr netaddr, broadcast; + + if (virSocketAddrBroadcast(network, &netmask, &broadcast) < 0 || + virSocketAddrMask(network, &netmask, &netaddr) < 0) { + virReportError(VIR_ERR_INTERNAL_ERROR, + _("failed to construct broadcast or network " + "address for network %s/%d"), + netStr, prefix); + goto error; + } + + /* Don't allow the start of the range to be the network + * address (usually "...0") or the end of the range to be the + * broadcast address (usually "...255"). (the opposite also + * isn't allowed, but checking for that is implicit in all the + * other combined checks) (IPv6 doesn't have broadcast and + * network addresses, so this check is only done for IPv4) + */ + if (virSocketAddrEqual(start, &netaddr)) { + virReportError(VIR_ERR_INTERNAL_ERROR, + _("start of range %s - %s in network %s/%d " + "is the network address"), + startStr, endStr, netStr, prefix); + goto error; + } + + if (virSocketAddrEqual(end, &broadcast)) { + virReportError(VIR_ERR_INTERNAL_ERROR, + _("end of range %s - %s in network %s/%d " + "is the broadcast address"), + startStr, endStr, netStr, prefix); + goto error; + } + } + } + + if (VIR_SOCKET_ADDR_IS_FAMILY(start, AF_INET)) { + virSocketAddrIPv4 t1, t2; + + if (virSocketAddrGetIPv4Addr(start, &t1) < 0 || + virSocketAddrGetIPv4Addr(end, &t2) < 0) { virReportError(VIR_ERR_INTERNAL_ERROR, _("failed to get IPv4 address " - "for start or end of range %s - %s " - "in network %s/%d"), - startStr, endStr, netStr, prefix); + "for start or end of range %s - %s"), + startStr, endStr); goto error; } - /* legacy check that everything except the last two bytes are - * the same + /* legacy check that everything except the last two bytes + * are the same */ for (i = 0; i < 2; i++) { if (t1[i] != t2[i]) { virReportError(VIR_ERR_INTERNAL_ERROR, - _("range %s - %s is too large (> 65535) " - "in network %s/%d"), - startStr, endStr, netStr, prefix); + _("range %s - %s is too large (> 65535)"), + startStr, endStr); goto error; } } ret = (t2[2] - t1[2]) * 256 + (t2[3] - t1[3]); if (ret < 0) { virReportError(VIR_ERR_INTERNAL_ERROR, - _("range %s - %s is reversed " - "in network %s/%d"), - startStr, endStr, netStr, prefix); + _("range %s - %s is reversed "), + startStr, endStr); goto error; } ret++; } else if (VIR_SOCKET_ADDR_IS_FAMILY(start, AF_INET6)) { virSocketAddrIPv6 t1, t2; - if ((virSocketAddrGetIPv6Addr(start, &t1) < 0) || - (virSocketAddrGetIPv6Addr(end, &t2) < 0)) { + if (virSocketAddrGetIPv6Addr(start, &t1) < 0 || + virSocketAddrGetIPv6Addr(end, &t2) < 0) { virReportError(VIR_ERR_INTERNAL_ERROR, _("failed to get IPv6 address " - "for start or end of range %s - %s " - "in network %s/%d"), - startStr, endStr, netStr, prefix); + "for start or end of range %s - %s"), + startStr, endStr); goto error; } @@ -757,29 +767,27 @@ virSocketAddrGetRange(virSocketAddrPtr start, virSocketAddrPtr end, for (i = 0; i < 7; i++) { if (t1[i] != t2[i]) { virReportError(VIR_ERR_INTERNAL_ERROR, - _("range %s - %s is too large (> 65535) " - "in network %s/%d"), - startStr, endStr, netStr, prefix); + _("range %s - %s is too large (> 65535)"), + startStr, endStr); goto error; } } ret = t2[7] - t1[7]; if (ret < 0) { virReportError(VIR_ERR_INTERNAL_ERROR, - _("range %s - %s start larger than end " - "in network %s/%d"), - startStr, endStr, netStr, prefix); + _("range %s - %s start larger than end"), + startStr, endStr); goto error; } ret++; } else { virReportError(VIR_ERR_INTERNAL_ERROR, _("unsupported address family " - "for range %s - %s " - "in network %s/%d, must be ipv4 or ipv6"), - startStr, endStr, netStr, prefix); + "for range %s - %s, must be ipv4 or ipv6"), + startStr, endStr); goto error; } + cleanup: VIR_FREE(startStr); VIR_FREE(endStr); diff --git a/tests/sockettest.c b/tests/sockettest.c index 292edb6..8f46218 100644 --- a/tests/sockettest.c +++ b/tests/sockettest.c @@ -98,10 +98,11 @@ testRange(const char *saddrstr, const char *eaddrstr, return -1; if (virSocketAddrParse(&eaddr, eaddrstr, AF_UNSPEC) < 0) return -1; - if (virSocketAddrParse(&netaddr, netstr, AF_UNSPEC) < 0) + if (netstr && virSocketAddrParse(&netaddr, netstr, AF_UNSPEC) < 0) return -1; - int gotsize = virSocketAddrGetRange(&saddr, &eaddr, &netaddr, prefix); + int gotsize = virSocketAddrGetRange(&saddr, &eaddr, + netstr ? &netaddr : NULL, prefix); VIR_DEBUG("Size want %d vs got %d", size, gotsize); if (pass) { /* fail if virSocketAddrGetRange returns failure, or unexpected size */ @@ -311,6 +312,15 @@ mymain(void) ret = -1; \ } while (0) +#define DO_TEST_RANGE_SIMPLE(saddr, eaddr, size, pass) \ + do { \ + struct testRangeData data \ + = { saddr, eaddr, NULL, 0, size, pass }; \ + if (virtTestRun("Test range " saddr " -> " eaddr "size " #size, \ + testRangeHelper, &data) < 0) \ + ret = -1; \ + } while (0) + #define DO_TEST_NETMASK(addr1, addr2, netmask, pass) \ do { \ struct testNetmaskData data = { addr1, addr2, netmask, pass }; \ @@ -373,23 +383,55 @@ mymain(void) DO_TEST_PARSE_AND_FORMAT("::1", AF_UNIX, false); DO_TEST_PARSE_AND_FORMAT("::ffff", AF_UNSPEC, true); + /* tests that specify a network that should contain the range */ DO_TEST_RANGE("192.168.122.1", "192.168.122.1", "192.168.122.1", 24, 1, true); DO_TEST_RANGE("192.168.122.1", "192.168.122.20", "192.168.122.22", 24, 20, true); + /* start of range is "network address" */ DO_TEST_RANGE("192.168.122.0", "192.168.122.254", "192.168.122.1", 24, -1, false); + /* end of range is "broadcast address" */ DO_TEST_RANGE("192.168.122.1", "192.168.122.255", "192.168.122.1", 24, -1, false); DO_TEST_RANGE("192.168.122.0", "192.168.122.255", "192.168.122.1", 16, 256, true); + /* range is reversed */ DO_TEST_RANGE("192.168.122.20", "192.168.122.1", "192.168.122.1", 24, -1, false); + /* start address outside network */ DO_TEST_RANGE("10.0.0.1", "192.168.122.20", "192.168.122.1", 24, -1, false); + /* end address outside network and range reversed */ DO_TEST_RANGE("192.168.122.20", "10.0.0.1", "192.168.122.1", 24, -1, false); + /* entire range outside network */ DO_TEST_RANGE("172.16.0.50", "172.16.0.254", "1.2.3.4", 8, -1, false); + /* end address outside network */ DO_TEST_RANGE("192.168.122.1", "192.168.123.20", "192.168.122.22", 24, -1, false); DO_TEST_RANGE("192.168.122.1", "192.168.123.20", "192.168.122.22", 23, 276, true); DO_TEST_RANGE("2000::1", "2000::1", "2000::1", 64, 1, true); DO_TEST_RANGE("2000::1", "2000::2", "2000::1", 64, 2, true); + /* range reversed */ DO_TEST_RANGE("2000::2", "2000::1", "2000::1", 64, -1, false); + /* range too large (> 65536) */ DO_TEST_RANGE("2000::1", "9001::1", "2000::1", 64, -1, false); + /* tests that *don't* specify a containing network + * (so fewer things can be checked) + */ + DO_TEST_RANGE_SIMPLE("192.168.122.1", "192.168.122.1", 1, true); + DO_TEST_RANGE_SIMPLE("192.168.122.1", "192.168.122.20", 20, true); + DO_TEST_RANGE_SIMPLE("192.168.122.0", "192.168.122.255", 256, true); + /* range is reversed */ + DO_TEST_RANGE_SIMPLE("192.168.122.20", "192.168.122.1", -1, false); + /* range too large (> 65536) */ + DO_TEST_RANGE_SIMPLE("10.0.0.1", "192.168.122.20", -1, false); + /* range reversed */ + DO_TEST_RANGE_SIMPLE("192.168.122.20", "10.0.0.1", -1, false); + DO_TEST_RANGE_SIMPLE("172.16.0.50", "172.16.0.254", 205, true); + DO_TEST_RANGE_SIMPLE("192.168.122.1", "192.168.123.20", 276, true); + + DO_TEST_RANGE_SIMPLE("2000::1", "2000::1", 1, true); + DO_TEST_RANGE_SIMPLE("2000::1", "2000::2", 2, true); + /* range reversed */ + DO_TEST_RANGE_SIMPLE("2000::2", "2000::1", -1, false); + /* range too large (> 65536) */ + DO_TEST_RANGE_SIMPLE("2000::1", "9001::1", -1, false); + DO_TEST_NETMASK("192.168.122.1", "192.168.122.2", "255.255.255.0", true); DO_TEST_NETMASK("192.168.122.1", "192.168.122.4", -- 2.1.0

3 3

[libvirt] [PATCH 0/9] Add missing QoS implementation
by Michal Privoznik 11 Aug '15

11 Aug '15

The first patch is unrelated to the rest, but I have it on the same branch. The patches 2..5 fix a daemon crasher (since domain_write ACL is required, I'm not going through libvirt-security). Then, since after patch 5 we have nearly everything prepared, patches 6..9 implement new feature: setting @floor live on domain interfaces. Michal Privoznik (9): virNetDevParseMcast: Avoid magic constant virNetDevBandwidthUpdateRate: turn class_id into integer bridge_driver: Introduce networkBandwidthChangeAllowed bridge_driver: Introduce networkBandwidthUpdate qemuDomainSetInterfaceParameters: Use new functions to update bandwidth virsh: Rework parseRateStr Introduce VIR_DOMAIN_BANDWIDTH_IN_FLOOR virsh: Implement VIR_DOMAIN_BANDWIDTH_IN_FLOOR qemu: Implement VIR_DOMAIN_BANDWIDTH_IN_FLOOR include/libvirt/libvirt-domain.h | 7 ++ src/network/bridge_driver.c | 234 +++++++++++++++++++++++++++++++++------ src/network/bridge_driver.h | 22 ++++ src/qemu/qemu_driver.c | 30 ++++- src/util/virnetdev.c | 2 +- src/util/virnetdevbandwidth.c | 10 +- src/util/virnetdevbandwidth.h | 2 +- tools/virsh-domain.c | 99 +++++++++++------ tools/virsh.pod | 12 +- 9 files changed, 334 insertions(+), 84 deletions(-) -- 2.4.6

2 26

[libvirt] [PATCH 0/6] admin API: Introduce server listing API
by Erik Skultety 11 Aug '15

11 Aug '15

I'll send virt-admin list call once the virsh split series is merged. Erik Skultety (6): test: Replace tabs with spaces in input-data-admin-nomdns.json rpc: Introduce new elements 'id' and 'name' to virnetserver structure admin: Drop 'internal.h' include from libvirt-admin.h admin: Move admin_server.{h,c} to admin.{h,c} admin: Introduce adminDaemonConnectListServers API admin: Usage example of the new server listing API .gitignore | 1 + Makefile.am | 2 +- configure.ac | 1 + daemon/Makefile.am | 4 +- daemon/admin.c | 174 +++++++++++++++ daemon/admin.h | 36 +++ daemon/admin_server.c | 116 ++++------ daemon/admin_server.h | 23 +- daemon/libvirtd.c | 6 +- examples/admin/Makefile.am | 25 +++ examples/admin/listservers.c | 73 ++++++ include/libvirt/libvirt-admin.h | 14 +- po/POTFILES.in | 2 +- src/admin/admin_protocol.x | 27 ++- src/datatypes.c | 35 +++ src/datatypes.h | 35 +++ src/libvirt-admin.c | 173 +++++++++++++++ src/libvirt_admin.syms | 4 + src/locking/lock_daemon.c | 2 +- src/lxc/lxc_controller.c | 2 +- src/rpc/virnetdaemon.c | 16 ++ src/rpc/virnetdaemon.h | 2 + src/rpc/virnetserver.c | 41 +++- src/rpc/virnetserver.h | 4 + .../virnetdaemondata/input-data-admin-nomdns.json | 246 +++++++++++---------- .../virnetdaemondata/input-data-anon-clients.json | 1 + .../input-data-initial-nomdns.json | 1 + tests/virnetdaemondata/input-data-initial.json | 1 + tests/virnetdaemontest.c | 2 +- 29 files changed, 846 insertions(+), 223 deletions(-) create mode 100644 daemon/admin.c create mode 100644 daemon/admin.h create mode 100644 examples/admin/Makefile.am create mode 100644 examples/admin/listservers.c -- 2.4.3

2 13

[libvirt] [PATCH v2 0/2] Add support for virtio-scsi controllers with ioeventfd
by Martin Kletzander 10 Aug '15

10 Aug '15

Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1150484 v2: - Changed one occurence virTristateBool to virTristateSwitch as it should be - wrapped one line to be <= 80 characters long - Rebased on top of current master (with laine's patches pushed) Martin Kletzander (2): conf: Add ioeventfd option for controllers qemu: Enable ioeventfd usage for virtio-scsi controllers docs/formatdomain.html.in | 8 ++++++ docs/schemas/domaincommon.rng | 3 +++ src/conf/domain_conf.c | 20 +++++++++++++-- src/conf/domain_conf.h | 1 + src/qemu/qemu_command.c | 7 ++++++ .../qemuxml2argv-disk-virtio-scsi-ioeventfd.args | 9 +++++++ .../qemuxml2argv-disk-virtio-scsi-ioeventfd.xml | 29 ++++++++++++++++++++++ tests/qemuxml2argvtest.c | 3 +++ tests/qemuxml2xmltest.c | 1 + 9 files changed, 79 insertions(+), 2 deletions(-) create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-disk-virtio-scsi-ioeventfd.args create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-disk-virtio-scsi-ioeventfd.xml -- 2.5.0

2 3