- Devel - libvirt List Archives

[libvirt] [PATCH] esx: Parse the path of the URI
by Matthias Bolte 03 Aug '10

03 Aug '10

The path will be used to specify the datacenter, compute resource and host system to be used with a vpx:// connection. --- src/esx/esx_driver.c | 49 ++++++++++------------- src/esx/esx_util.c | 104 +++++++++++++++++++++++++++++++------------------- src/esx/esx_util.h | 11 +++-- src/esx/esx_vi.c | 14 +++--- src/esx/esx_vi.h | 3 +- 5 files changed, 102 insertions(+), 79 deletions(-) diff --git a/src/esx/esx_driver.c b/src/esx/esx_driver.c index 913420c..3bdc551 100644 --- a/src/esx/esx_driver.c +++ b/src/esx/esx_driver.c @@ -294,7 +294,7 @@ static int esxConnectToHost(esxPrivate *priv, virConnectAuthPtr auth, const char *hostname, int port, const char *predefinedUsername, - esxUtil_ParsedQuery *parsedQuery, + esxUtil_ParsedUri *parsedUri, esxVI_ProductVersion expectedProductVersion, char **vCenterIpAddress) { @@ -347,7 +347,7 @@ esxConnectToHost(esxPrivate *priv, virConnectAuthPtr auth, if (esxVI_Context_Alloc(&priv->host) < 0 || esxVI_Context_Connect(priv->host, url, ipAddress, username, password, - parsedQuery) < 0) { + parsedUri) < 0) { goto cleanup; } @@ -416,7 +416,7 @@ static int esxConnectToVCenter(esxPrivate *priv, virConnectAuthPtr auth, const char *hostname, int port, const char *predefinedUsername, - esxUtil_ParsedQuery *parsedQuery) + esxUtil_ParsedUri *parsedUri) { int result = -1; char ipAddress[NI_MAXHOST] = ""; @@ -459,7 +459,7 @@ esxConnectToVCenter(esxPrivate *priv, virConnectAuthPtr auth, if (esxVI_Context_Alloc(&priv->vCenter) < 0 || esxVI_Context_Connect(priv->vCenter, url, ipAddress, username, - password, parsedQuery) < 0) { + password, parsedUri) < 0) { goto cleanup; } @@ -528,7 +528,7 @@ esxOpen(virConnectPtr conn, virConnectAuthPtr auth, int flags ATTRIBUTE_UNUSED) { virDrvOpenStatus result = VIR_DRV_OPEN_ERROR; esxPrivate *priv = NULL; - esxUtil_ParsedQuery *parsedQuery = NULL; + esxUtil_ParsedUri *parsedUri = NULL; char *potentialVCenterIpAddress = NULL; char vCenterIpAddress[NI_MAXHOST] = ""; @@ -545,29 +545,24 @@ esxOpen(virConnectPtr conn, virConnectAuthPtr auth, int flags ATTRIBUTE_UNUSED) return VIR_DRV_OPEN_DECLINED; } - if (conn->uri->path != NULL && STRNEQ(conn->uri->path, "") && - STRNEQ(conn->uri->path, "/")) { - VIR_WARN("Ignoring unexpected path '%s' in URI", conn->uri->path); - } - /* Allocate per-connection private data */ if (VIR_ALLOC(priv) < 0) { virReportOOMError(); goto cleanup; } - if (esxUtil_ParseQuery(&parsedQuery, conn->uri) < 0) { + if (esxUtil_ParseUri(&parsedUri, conn->uri) < 0) { goto cleanup; } - priv->transport = parsedQuery->transport; - parsedQuery->transport = NULL; + priv->transport = parsedUri->transport; + parsedUri->transport = NULL; priv->maxVcpus = -1; priv->supportsVMotion = esxVI_Boolean_Undefined; priv->supportsLongMode = esxVI_Boolean_Undefined; - priv->autoAnswer = parsedQuery->autoAnswer ? esxVI_Boolean_True - : esxVI_Boolean_False; + priv->autoAnswer = parsedUri->autoAnswer ? esxVI_Boolean_True + : esxVI_Boolean_False; priv->usedCpuTimeCounterId = -1; /* @@ -597,7 +592,7 @@ esxOpen(virConnectPtr conn, virConnectAuthPtr auth, int flags ATTRIBUTE_UNUSED) STRCASEEQ(conn->uri->scheme, "gsx")) { /* Connect to host */ if (esxConnectToHost(priv, auth, conn->uri->server, conn->uri->port, - conn->uri->user, parsedQuery, + conn->uri->user, parsedUri, STRCASEEQ(conn->uri->scheme, "esx") ? esxVI_ProductVersion_ESX : esxVI_ProductVersion_GSX, @@ -606,8 +601,8 @@ esxOpen(virConnectPtr conn, virConnectAuthPtr auth, int flags ATTRIBUTE_UNUSED) } /* Connect to vCenter */ - if (parsedQuery->vCenter != NULL) { - if (STREQ(parsedQuery->vCenter, "*")) { + if (parsedUri->vCenter != NULL) { + if (STREQ(parsedUri->vCenter, "*")) { if (potentialVCenterIpAddress == NULL) { ESX_ERROR(VIR_ERR_INTERNAL_ERROR, "%s", _("This host is not managed by a vCenter")); @@ -622,7 +617,7 @@ esxOpen(virConnectPtr conn, virConnectAuthPtr auth, int flags ATTRIBUTE_UNUSED) goto cleanup; } } else { - if (esxUtil_ResolveHostname(parsedQuery->vCenter, + if (esxUtil_ResolveHostname(parsedUri->vCenter, vCenterIpAddress, NI_MAXHOST) < 0) { goto cleanup; } @@ -633,14 +628,14 @@ esxOpen(virConnectPtr conn, virConnectAuthPtr auth, int flags ATTRIBUTE_UNUSED) _("This host is managed by a vCenter with IP " "address %s, but a mismachting vCenter '%s' " "(%s) has been specified"), - potentialVCenterIpAddress, parsedQuery->vCenter, + potentialVCenterIpAddress, parsedUri->vCenter, vCenterIpAddress); goto cleanup; } } if (esxConnectToVCenter(priv, auth, vCenterIpAddress, - conn->uri->port, NULL, parsedQuery) < 0) { + conn->uri->port, NULL, parsedUri) < 0) { goto cleanup; } } @@ -649,7 +644,7 @@ esxOpen(virConnectPtr conn, virConnectAuthPtr auth, int flags ATTRIBUTE_UNUSED) } else { /* VPX */ /* Connect to vCenter */ if (esxConnectToVCenter(priv, auth, conn->uri->server, conn->uri->port, - conn->uri->user, parsedQuery) < 0) { + conn->uri->user, parsedUri) < 0) { goto cleanup; } @@ -678,7 +673,7 @@ esxOpen(virConnectPtr conn, virConnectAuthPtr auth, int flags ATTRIBUTE_UNUSED) VIR_FREE(priv); } - esxUtil_FreeParsedQuery(&parsedQuery); + esxUtil_FreeParsedUri(&parsedUri); VIR_FREE(potentialVCenterIpAddress); return result; @@ -3113,14 +3108,14 @@ esxDomainMigratePrepare(virConnectPtr dconn, unsigned long resource ATTRIBUTE_UNUSED) { int result = -1; - esxUtil_ParsedQuery *parsedQuery = NULL; + esxUtil_ParsedUri *parsedUri = NULL; if (uri_in == NULL) { - if (esxUtil_ParseQuery(&parsedQuery, dconn->uri) < 0) { + if (esxUtil_ParseUri(&parsedUri, dconn->uri) < 0) { return -1; } - if (virAsprintf(uri_out, "%s://%s:%d/sdk", parsedQuery->transport, + if (virAsprintf(uri_out, "%s://%s:%d/sdk", parsedUri->transport, dconn->uri->server, dconn->uri->port) < 0) { virReportOOMError(); goto cleanup; @@ -3130,7 +3125,7 @@ esxDomainMigratePrepare(virConnectPtr dconn, result = 0; cleanup: - esxUtil_FreeParsedQuery(&parsedQuery); + esxUtil_FreeParsedUri(&parsedUri); return result; } diff --git a/src/esx/esx_util.c b/src/esx/esx_util.c index d79de2c..75a9aaf 100644 --- a/src/esx/esx_util.c +++ b/src/esx/esx_util.c @@ -41,7 +41,7 @@ int -esxUtil_ParseQuery(esxUtil_ParsedQuery **parsedQuery, xmlURIPtr uri) +esxUtil_ParseUri(esxUtil_ParsedUri **parsedUri, xmlURIPtr uri) { int result = -1; struct qparam_set *queryParamSet = NULL; @@ -50,13 +50,14 @@ esxUtil_ParseQuery(esxUtil_ParsedQuery **parsedQuery, xmlURIPtr uri) int noVerify; int autoAnswer; char *tmp; + char *saveptr; - if (parsedQuery == NULL || *parsedQuery != NULL) { + if (parsedUri == NULL || *parsedUri != NULL) { ESX_VI_ERROR(VIR_ERR_INTERNAL_ERROR, "%s", _("Invalid argument")); return -1; } - if (VIR_ALLOC(*parsedQuery) < 0) { + if (VIR_ALLOC(*parsedUri) < 0) { virReportOOMError(); return -1; } @@ -75,29 +76,29 @@ esxUtil_ParseQuery(esxUtil_ParsedQuery **parsedQuery, xmlURIPtr uri) queryParam = &queryParamSet->p[i]; if (STRCASEEQ(queryParam->name, "transport")) { - VIR_FREE((*parsedQuery)->transport); + VIR_FREE((*parsedUri)->transport); - (*parsedQuery)->transport = strdup(queryParam->value); + (*parsedUri)->transport = strdup(queryParam->value); - if ((*parsedQuery)->transport == NULL) { + if ((*parsedUri)->transport == NULL) { virReportOOMError(); goto cleanup; } - if (STRNEQ((*parsedQuery)->transport, "http") && - STRNEQ((*parsedQuery)->transport, "https")) { + if (STRNEQ((*parsedUri)->transport, "http") && + STRNEQ((*parsedUri)->transport, "https")) { ESX_ERROR(VIR_ERR_INVALID_ARG, _("Query parameter 'transport' has unexpected value " "'%s' (should be http|https)"), - (*parsedQuery)->transport); + (*parsedUri)->transport); goto cleanup; } } else if (STRCASEEQ(queryParam->name, "vcenter")) { - VIR_FREE((*parsedQuery)->vCenter); + VIR_FREE((*parsedUri)->vCenter); - (*parsedQuery)->vCenter = strdup(queryParam->value); + (*parsedUri)->vCenter = strdup(queryParam->value); - if ((*parsedQuery)->vCenter == NULL) { + if ((*parsedUri)->vCenter == NULL) { virReportOOMError(); goto cleanup; } @@ -110,7 +111,7 @@ esxUtil_ParseQuery(esxUtil_ParsedQuery **parsedQuery, xmlURIPtr uri) goto cleanup; } - (*parsedQuery)->noVerify = noVerify != 0; + (*parsedUri)->noVerify = noVerify != 0; } else if (STRCASEEQ(queryParam->name, "auto_answer")) { if (virStrToLong_i(queryParam->value, NULL, 10, &autoAnswer) < 0 || (autoAnswer != 0 && autoAnswer != 1)) { @@ -120,23 +121,23 @@ esxUtil_ParseQuery(esxUtil_ParsedQuery **parsedQuery, xmlURIPtr uri) goto cleanup; } - (*parsedQuery)->autoAnswer = autoAnswer != 0; + (*parsedUri)->autoAnswer = autoAnswer != 0; } else if (STRCASEEQ(queryParam->name, "proxy")) { /* Expected format: [<type>://]<hostname>[:<port>] */ - (*parsedQuery)->proxy = true; - (*parsedQuery)->proxy_type = CURLPROXY_HTTP; - VIR_FREE((*parsedQuery)->proxy_hostname); - (*parsedQuery)->proxy_port = 1080; + (*parsedUri)->proxy = true; + (*parsedUri)->proxy_type = CURLPROXY_HTTP; + VIR_FREE((*parsedUri)->proxy_hostname); + (*parsedUri)->proxy_port = 1080; if ((tmp = STRSKIP(queryParam->value, "http://")) != NULL) { - (*parsedQuery)->proxy_type = CURLPROXY_HTTP; + (*parsedUri)->proxy_type = CURLPROXY_HTTP; } else if ((tmp = STRSKIP(queryParam->value, "socks://")) != NULL || (tmp = STRSKIP(queryParam->value, "socks5://")) != NULL) { - (*parsedQuery)->proxy_type = CURLPROXY_SOCKS5; + (*parsedUri)->proxy_type = CURLPROXY_SOCKS5; } else if ((tmp = STRSKIP(queryParam->value, "socks4://")) != NULL) { - (*parsedQuery)->proxy_type = CURLPROXY_SOCKS4; + (*parsedUri)->proxy_type = CURLPROXY_SOCKS4; } else if ((tmp = STRSKIP(queryParam->value, "socks4a://")) != NULL) { - (*parsedQuery)->proxy_type = CURLPROXY_SOCKS4A; + (*parsedUri)->proxy_type = CURLPROXY_SOCKS4A; } else if ((tmp = strstr(queryParam->value, "://")) != NULL) { *tmp = '\0'; @@ -149,15 +150,15 @@ esxUtil_ParseQuery(esxUtil_ParsedQuery **parsedQuery, xmlURIPtr uri) tmp = queryParam->value; } - (*parsedQuery)->proxy_hostname = strdup(tmp); + (*parsedUri)->proxy_hostname = strdup(tmp); - if ((*parsedQuery)->proxy_hostname == NULL) { + if ((*parsedUri)->proxy_hostname == NULL) { virReportOOMError(); goto cleanup; } - if ((tmp = strchr((*parsedQuery)->proxy_hostname, ':')) != NULL) { - if (tmp == (*parsedQuery)->proxy_hostname) { + if ((tmp = strchr((*parsedUri)->proxy_hostname, ':')) != NULL) { + if (tmp == (*parsedUri)->proxy_hostname) { ESX_ERROR(VIR_ERR_INVALID_ARG, "%s", _("Query parameter 'proxy' doesn't contain a " "hostname")); @@ -167,9 +168,9 @@ esxUtil_ParseQuery(esxUtil_ParsedQuery **parsedQuery, xmlURIPtr uri) *tmp++ = '\0'; if (virStrToLong_i(tmp, NULL, 10, - &(*parsedQuery)->proxy_port) < 0 || - (*parsedQuery)->proxy_port < 1 || - (*parsedQuery)->proxy_port > 65535) { + &(*parsedUri)->proxy_port) < 0 || + (*parsedUri)->proxy_port < 1 || + (*parsedUri)->proxy_port > 65535) { ESX_ERROR(VIR_ERR_INVALID_ARG, _("Query parameter 'proxy' has unexpected port" "value '%s' (should be [1..65535])"), tmp); @@ -182,10 +183,32 @@ esxUtil_ParseQuery(esxUtil_ParsedQuery **parsedQuery, xmlURIPtr uri) } } - if ((*parsedQuery)->transport == NULL) { - (*parsedQuery)->transport = strdup("https"); + /* Expected format: [/]<datacenter>/<computeresource>[/<hostsystem>] */ + if (uri->path != NULL) { + tmp = strdup(uri->path); - if ((*parsedQuery)->transport == NULL) { + if (tmp == NULL) { + virReportOOMError(); + goto cleanup; + } + + if (esxVI_String_DeepCopyValue(&(*parsedUri)->path_datacenter, + strtok_r(tmp, "/", &saveptr)) < 0 || + esxVI_String_DeepCopyValue(&(*parsedUri)->path_computeResource, + strtok_r(NULL, "/", &saveptr)) < 0 || + esxVI_String_DeepCopyValue(&(*parsedUri)->path_hostSystem, + strtok_r(NULL, "", &saveptr)) < 0) { + VIR_FREE(tmp); + goto cleanup; + } + + VIR_FREE(tmp); + } + + if ((*parsedUri)->transport == NULL) { + (*parsedUri)->transport = strdup("https"); + + if ((*parsedUri)->transport == NULL) { virReportOOMError(); goto cleanup; } @@ -195,7 +218,7 @@ esxUtil_ParseQuery(esxUtil_ParsedQuery **parsedQuery, xmlURIPtr uri) cleanup: if (result < 0) { - esxUtil_FreeParsedQuery(parsedQuery); + esxUtil_FreeParsedUri(parsedUri); } if (queryParamSet != NULL) { @@ -209,17 +232,20 @@ esxUtil_ParseQuery(esxUtil_ParsedQuery **parsedQuery, xmlURIPtr uri) void -esxUtil_FreeParsedQuery(esxUtil_ParsedQuery **parsedQuery) +esxUtil_FreeParsedUri(esxUtil_ParsedUri **parsedUri) { - if (parsedQuery == NULL || *parsedQuery == NULL) { + if (parsedUri == NULL || *parsedUri == NULL) { return; } - VIR_FREE((*parsedQuery)->transport); - VIR_FREE((*parsedQuery)->vCenter); - VIR_FREE((*parsedQuery)->proxy_hostname); + VIR_FREE((*parsedUri)->transport); + VIR_FREE((*parsedUri)->vCenter); + VIR_FREE((*parsedUri)->proxy_hostname); + VIR_FREE((*parsedUri)->path_datacenter); + VIR_FREE((*parsedUri)->path_computeResource); + VIR_FREE((*parsedUri)->path_hostSystem); - VIR_FREE(*parsedQuery); + VIR_FREE(*parsedUri); } diff --git a/src/esx/esx_util.h b/src/esx/esx_util.h index a1927a6..99ce81d 100644 --- a/src/esx/esx_util.h +++ b/src/esx/esx_util.h @@ -29,9 +29,9 @@ # include "internal.h" # include "conf.h" -typedef struct _esxUtil_ParsedQuery esxUtil_ParsedQuery; +typedef struct _esxUtil_ParsedUri esxUtil_ParsedUri; -struct _esxUtil_ParsedQuery { +struct _esxUtil_ParsedUri { char *transport; char *vCenter; bool noVerify; @@ -40,11 +40,14 @@ struct _esxUtil_ParsedQuery { int proxy_type; char *proxy_hostname; int proxy_port; + char *path_datacenter; + char *path_computeResource; + char *path_hostSystem; }; -int esxUtil_ParseQuery(esxUtil_ParsedQuery **parsedQuery, xmlURIPtr uri); +int esxUtil_ParseUri(esxUtil_ParsedUri **parsedUri, xmlURIPtr uri); -void esxUtil_FreeParsedQuery(esxUtil_ParsedQuery **parsedQuery); +void esxUtil_FreeParsedUri(esxUtil_ParsedUri **parsedUri); int esxUtil_ParseVirtualMachineIDString(const char *id_string, int *id); diff --git a/src/esx/esx_vi.c b/src/esx/esx_vi.c index 98b8bcd..5695881 100644 --- a/src/esx/esx_vi.c +++ b/src/esx/esx_vi.c @@ -277,7 +277,7 @@ esxVI_CURL_Perform(esxVI_Context *ctx, const char *url) int esxVI_Context_Connect(esxVI_Context *ctx, const char *url, const char *ipAddress, const char *username, - const char *password, esxUtil_ParsedQuery *parsedQuery) + const char *password, esxUtil_ParsedUri *parsedUri) { int result = -1; esxVI_String *propertyNameList = NULL; @@ -329,9 +329,9 @@ esxVI_Context_Connect(esxVI_Context *ctx, const char *url, curl_easy_setopt(ctx->curl_handle, CURLOPT_HEADER, 0); curl_easy_setopt(ctx->curl_handle, CURLOPT_FOLLOWLOCATION, 0); curl_easy_setopt(ctx->curl_handle, CURLOPT_SSL_VERIFYPEER, - parsedQuery->noVerify ? 0 : 1); + parsedUri->noVerify ? 0 : 1); curl_easy_setopt(ctx->curl_handle, CURLOPT_SSL_VERIFYHOST, - parsedQuery->noVerify ? 0 : 2); + parsedUri->noVerify ? 0 : 2); curl_easy_setopt(ctx->curl_handle, CURLOPT_COOKIEFILE, ""); curl_easy_setopt(ctx->curl_handle, CURLOPT_HTTPHEADER, ctx->curl_headers); curl_easy_setopt(ctx->curl_handle, CURLOPT_READFUNCTION, @@ -345,13 +345,13 @@ esxVI_Context_Connect(esxVI_Context *ctx, const char *url, curl_easy_setopt(ctx->curl_handle, CURLOPT_VERBOSE, 1); #endif - if (parsedQuery->proxy) { + if (parsedUri->proxy) { curl_easy_setopt(ctx->curl_handle, CURLOPT_PROXY, - parsedQuery->proxy_hostname); + parsedUri->proxy_hostname); curl_easy_setopt(ctx->curl_handle, CURLOPT_PROXYTYPE, - parsedQuery->proxy_type); + parsedUri->proxy_type); curl_easy_setopt(ctx->curl_handle, CURLOPT_PROXYPORT, - parsedQuery->proxy_port); + parsedUri->proxy_port); } if (virMutexInit(&ctx->curl_lock) < 0) { diff --git a/src/esx/esx_vi.h b/src/esx/esx_vi.h index ef9c6d9..325ba69 100644 --- a/src/esx/esx_vi.h +++ b/src/esx/esx_vi.h @@ -168,8 +168,7 @@ int esxVI_Context_Alloc(esxVI_Context **ctx); void esxVI_Context_Free(esxVI_Context **ctx); int esxVI_Context_Connect(esxVI_Context *ctx, const char *ipAddress, const char *url, const char *username, - const char *password, - esxUtil_ParsedQuery *parsedQuery); + const char *password, esxUtil_ParsedUri *parsedUri); int esxVI_Context_DownloadFile(esxVI_Context *ctx, const char *url, char **content); int esxVI_Context_UploadFile(esxVI_Context *ctx, const char *url, -- 1.7.0.4

2 2

[libvirt] [PATCH] Add iptables rule to fixup DHCP response checksum.
by Laine Stump 03 Aug '10

03 Aug '10

From: Laine Stump <laine(a)redhat.com> (I don't know whether or not we want to commit this upstream yet - the proposed iptables and kernel module backend for the changes have been posted but not yet committed upstream. On the other hand, the new libvirt code ends up simply printing a warning message if the necessary iptables support isn't yet in place.) This patch attempts to take advantage of a newly added netfilter module to correct for a problem with some guest DHCP client implementations when used in conjunction with a DHCP server run on the host systems with packet checksum offloading enabled. The problem is that, when the guest uses a RAW socket to read the DHCP response packets, the checksum hasn't yet been fixed by the IP stack, so it is incorrect. The fix implemented here is to add a rule to the POSTROUTING chain of the mangle table in iptables that fixes up the checksum for packets on the virtual network's bridge that are destined for the bootpc port (ie "dhcpc", ie port 68) port on the guest. Only very new versions of iptables will have this support (it has been submitted upstream, but not yet committed), so a failure to add this rule only results in a warning message. The iptables patch is here: http://patchwork.ozlabs.org/patch/58525/ A corresponding kernel module patch is also required (the backend of the iptables patch) and has been submitted, but I don't have the details for that (I tested using a pre-built image I received from the developer, Michael Tsirkin). --- src/libvirt_private.syms | 2 + src/network/bridge_driver.c | 17 ++++++++++ src/util/iptables.c | 71 +++++++++++++++++++++++++++++++++++++++++++ src/util/iptables.h | 6 ++++ 4 files changed, 96 insertions(+), 0 deletions(-) diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index 778ceb1..d81d4cf 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -328,6 +328,7 @@ iptablesAddForwardAllowRelatedIn; iptablesAddForwardMasquerade; iptablesAddForwardRejectIn; iptablesAddForwardRejectOut; +iptablesAddOutputFixUdpChecksum; iptablesAddTcpInput; iptablesAddUdpInput; iptablesContextFree; @@ -339,6 +340,7 @@ iptablesRemoveForwardAllowRelatedIn; iptablesRemoveForwardMasquerade; iptablesRemoveForwardRejectIn; iptablesRemoveForwardRejectOut; +iptablesRemoveOutputFixUdpChecksum; iptablesRemoveTcpInput; iptablesRemoveUdpInput; diff --git a/src/network/bridge_driver.c b/src/network/bridge_driver.c index 72255c1..c4480ff 100644 --- a/src/network/bridge_driver.c +++ b/src/network/bridge_driver.c @@ -781,6 +781,19 @@ networkAddIptablesRules(struct network_driver *driver, !networkAddRoutingIptablesRules(driver, network)) goto err8; + /* If we are doing local DHCP service on this network, attempt to + * add a rule that will fixup the checksum of DHCP response + * packets back to the guests (but report failure without + * aborting, since not all iptables implementations support it). + */ + + if ((network->def->ipAddress || network->def->nranges) && + (iptablesAddOutputFixUdpChecksum(driver->iptables, + network->def->bridge, 68) != 0)) { + VIR_WARN("Could not add rule to fixup DHCP response checksums " + "on network '%s'", network->def->name); + } + return 1; err8: @@ -811,6 +824,10 @@ networkAddIptablesRules(struct network_driver *driver, static void networkRemoveIptablesRules(struct network_driver *driver, virNetworkObjPtr network) { + if (network->def->ipAddress || network->def->nranges) { + iptablesRemoveOutputFixUdpChecksum(driver->iptables, + network->def->bridge, 68); + } if (network->def->forwardType != VIR_NETWORK_FORWARD_NONE) { if (network->def->forwardType == VIR_NETWORK_FORWARD_NAT) { iptablesRemoveForwardMasquerade(driver->iptables, diff --git a/src/util/iptables.c b/src/util/iptables.c index d06b857..9b888e5 100644 --- a/src/util/iptables.c +++ b/src/util/iptables.c @@ -60,6 +60,7 @@ struct _iptablesContext iptRules *input_filter; iptRules *forward_filter; iptRules *nat_postrouting; + iptRules *mangle_postrouting; }; static void @@ -188,6 +189,9 @@ iptablesContextNew(void) if (!(ctx->nat_postrouting = iptRulesNew("nat", "POSTROUTING"))) goto error; + if (!(ctx->mangle_postrouting = iptRulesNew("mangle", "POSTROUTING"))) + goto error; + return ctx; error: @@ -210,6 +214,8 @@ iptablesContextFree(iptablesContext *ctx) iptRulesFree(ctx->forward_filter); if (ctx->nat_postrouting) iptRulesFree(ctx->nat_postrouting); + if (ctx->mangle_postrouting) + iptRulesFree(ctx->mangle_postrouting); VIR_FREE(ctx); } @@ -753,3 +759,68 @@ iptablesRemoveForwardMasquerade(iptablesContext *ctx, { return iptablesForwardMasquerade(ctx, network, physdev, REMOVE); } + + +static int +iptablesOutputFixUdpChecksum(iptablesContext *ctx, + const char *iface, + int port, + int action) +{ + char portstr[32]; + + snprintf(portstr, sizeof(portstr), "%d", port); + portstr[sizeof(portstr) - 1] = '\0'; + + return iptablesAddRemoveRule(ctx->mangle_postrouting, + action, + "--out-interface", iface, + "--protocol", "udp", + "--destination-port", portstr, + "--jump", "CHECKSUM", "--checksum-fill", + NULL); +} + +/** + * iptablesAddOutputFixUdpChecksum: + * @ctx: pointer to the IP table context + * @iface: the interface name + * @port: the UDP port to match + * + * Add an rule to the mangle table's POSTROUTING chain that fixes up the + * checksum of packets with the given destination @port. + * the given @iface interface for TCP packets. + * + * Returns 0 in case of success or an error code in case of error. + * (NB: if the system's iptables does not support checksum mangling, + * this will return an error, which should be ignored.) + */ + +int +iptablesAddOutputFixUdpChecksum(iptablesContext *ctx, + const char *iface, + int port) +{ + return iptablesOutputFixUdpChecksum(ctx, iface, port, ADD); +} + +/** + * iptablesRemoveOutputFixUdpChecksum: + * @ctx: pointer to the IP table context + * @iface: the interface name + * @port: the UDP port of the rule to remove + * + * Removes the checksum fixup rule that was previous added with + * iptablesAddOutputFixUdpChecksum. + * + * Returns 0 in case of success or an error code in case of error + * (again, if iptables doesn't support checksum fixup, this will + * return an error, which should be ignored) + */ +int +iptablesRemoveOutputFixUdpChecksum(iptablesContext *ctx, + const char *iface, + int port) +{ + return iptablesOutputFixUdpChecksum(ctx, iface, port, REMOVE); +} diff --git a/src/util/iptables.h b/src/util/iptables.h index 7d55a6d..5c2e553 100644 --- a/src/util/iptables.h +++ b/src/util/iptables.h @@ -89,5 +89,11 @@ int iptablesAddForwardMasquerade (iptablesContext *ctx, int iptablesRemoveForwardMasquerade (iptablesContext *ctx, const char *network, const char *physdev); +int iptablesAddOutputFixUdpChecksum (iptablesContext *ctx, + const char *iface, + int port); +int iptablesRemoveOutputFixUdpChecksum (iptablesContext *ctx, + const char *iface, + int port); #endif /* __QEMUD_IPTABLES_H__ */ -- 1.7.1.1

2 3

[libvirt] [PATCH] qemu: kill some dead stores
by Eric Blake 03 Aug '10

03 Aug '10

Spotted by clang. The qemuConnectMonitor one is an outright bug, the other two are cosmetic. * src/qemu/qemu_monitor.c (qemuMonitorClose): Kill dead store. * src/qemu/qemu_driver.c (qemudDomainSaveImageStartVM): Likewise. (qemuConnectMonitor): Don't lose error status. --- src/qemu/qemu_driver.c | 2 -- src/qemu/qemu_monitor.c | 4 +--- 2 files changed, 1 insertions(+), 5 deletions(-) diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 098f4da..57b8271 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -1416,7 +1416,6 @@ qemuConnectMonitor(struct qemud_driver *driver, virDomainObjPtr vm) ret = qemuMonitorSetCapabilities(priv->mon); qemuDomainObjExitMonitorWithDriver(driver, vm); - ret = 0; error: if (ret < 0) qemuMonitorClose(priv->mon); @@ -6535,7 +6534,6 @@ qemudDomainSaveImageStartVM(virConnectPtr conn, wait_ret = qemudDomainSaveImageClose(fd, read_pid, &status); fd = -1; if (read_pid != -1) { - read_pid = -1; if (wait_ret == -1) { virReportSystemError(errno, _("failed to wait for process reading '%s'"), diff --git a/src/qemu/qemu_monitor.c b/src/qemu/qemu_monitor.c index f1494ff..2366fdb 100644 --- a/src/qemu/qemu_monitor.c +++ b/src/qemu/qemu_monitor.c @@ -685,8 +685,6 @@ cleanup: void qemuMonitorClose(qemuMonitorPtr mon) { - int refs; - if (!mon) return; @@ -706,7 +704,7 @@ void qemuMonitorClose(qemuMonitorPtr mon) mon->closed = 1; } - if ((refs = qemuMonitorUnref(mon)) > 0) + if (qemuMonitorUnref(mon) > 0) qemuMonitorUnlock(mon); } -- 1.7.2

2 4

[libvirt] [PATCH] Check that virsh -d argument is numeric
by Daniel Veillard 03 Aug '10

03 Aug '10

Having been bitten one more time by the use of -d to pass the hypervisor URI instead of -c (confusion coming from CVS using -d to specify the root), I suggest to drop atoi and use the function with checking and error out with proper explanation instead of silently failing ! paphio:~/libvirt -> /usr/bin/virsh -d qemu+ssh://test2/system list Id Name State ---------------------------------- paphio:~/libvirt -> tools/virsh -d qemu+ssh://test2/system list error: option -d take a numeric argument paphio:~/libvirt -> tools/virsh -d 5 list command: "list " Id Name State ---------------------------------- paphio:~/libvirt -> Daniel -- Daniel Veillard | libxml Gnome XML XSLT toolkit http://xmlsoft.org/ daniel(a)veillard.com | Rpmfind RPM search engine http://rpmfind.net/ http://veillard.com/ | virtualization library http://libvirt.org/

3 6

[libvirt] [PATCH] Fix the ACS checking in the PCI code.
by Chris Lalancette 02 Aug '10

02 Aug '10

When trying to assign a PCI device to a guest, we have to check that all bridges upstream of that device support ACS. That means that we have to find the parent bridge of the current device, check for ACS, then find the parent bridge of that device, check for ACS, etc. As it currently stands, the code to do this iterates through all PCI devices on the system, looking for a device that has a range of busses that included the current device's bus. That check is not restrictive enough, though. Depending on how we iterated through the list of PCI devices, we could first find the *topmost* bridge in the system; since it necessarily had a range of busses including the current device's bus, we would only ever check the topmost bridge, and not check any of the intermediate bridges. Note that this also caused a fairly serious bug in the secondary bus reset code, where we could erroneously find and reset the topmost bus instead of the inner bus. This patch changes pciGetParentDevice() so that it first checks if a bridge device's secondary bus exactly matches the bus of the device we are looking for. If it does, we've found the correct parent bridge and we are done. If it does not, then we check to see if this bridge device's busses *include* the bus of the device we care about. If so, we mark this bridge device as best, and go on. If we later find another bridge device whose busses include this device, but is more restrictive, then we free up the previous best and mark the new one as best. This algorithm ensures that in the normal case we find the direct parent, but in the case that the parent bridge secondary bus is not exactly the same as the device, we still find the correct bridge. This patch was tested by me on a 4-port NIC with a bridge without ACS (where assignment failed), a 4-port NIC with a bridge with ACS (where assignment succeeded), and a 2-port NIC with no bridges (where assignment succeeded). Signed-off-by: Chris Lalancette <clalance(a)redhat.com> --- src/util/pci.c | 78 +++++++++++++++++++++++++++++++++++++++++++++++-------- 1 files changed, 66 insertions(+), 12 deletions(-) diff --git a/src/util/pci.c b/src/util/pci.c index 26d55b8..f2890bd 100644 --- a/src/util/pci.c +++ b/src/util/pci.c @@ -283,6 +283,7 @@ pciIterDevices(pciIterPredicate predicate, DIR *dir; struct dirent *entry; int ret = 0; + int rc; *matched = NULL; @@ -322,11 +323,20 @@ pciIterDevices(pciIterPredicate predicate, break; } - if (predicate(dev, check, data)) { + rc = predicate(dev, check, data); + if (rc < 0) { + /* the predicate returned an error, bail */ + pciFreeDevice(check); + ret = -1; + break; + } + else if (rc == 1) { VIR_DEBUG("%s %s: iter matched on %s", dev->id, dev->name, check->name); *matched = check; + ret = 1; break; } + pciFreeDevice(check); } closedir(dir); @@ -510,10 +520,11 @@ pciBusContainsActiveDevices(pciDevice *dev, /* Is @check the parent of @dev ? */ static int -pciIsParent(pciDevice *dev, pciDevice *check, void *data ATTRIBUTE_UNUSED) +pciIsParent(pciDevice *dev, pciDevice *check, void *data) { uint16_t device_class; uint8_t header_type, secondary, subordinate; + pciDevice **best = data; if (dev->domain != check->domain) return 0; @@ -533,16 +544,54 @@ pciIsParent(pciDevice *dev, pciDevice *check, void *data ATTRIBUTE_UNUSED) VIR_DEBUG("%s %s: found parent device %s", dev->id, dev->name, check->name); - /* No, it's superman! */ - return (dev->bus >= secondary && dev->bus <= subordinate); + /* if the secondary bus exactly equals the device's bus, then we found + * the direct parent. No further work is necessary + */ + if (dev->bus == secondary) + return 1; + + /* otherwise, SRIOV allows VFs to be on different busses then their PFs. + * In this case, what we need to do is look for the "best" match; i.e. + * the most restrictive match that still satisfies all of the conditions. + */ + if (dev->bus > secondary && dev->bus <= subordinate) { + if (*best == NULL) { + *best = pciGetDevice(check->domain, check->bus, check->slot, + check->function); + if (*best == NULL) + return -1; + } + else { + /* OK, we had already recorded a previous "best" match for the + * parent. See if the current device is more restrictive than the + * best, and if so, make it the new best + */ + if (secondary > pciRead8(*best, PCI_SECONDARY_BUS)) { + pciFreeDevice(*best); + *best = pciGetDevice(check->domain, check->bus, check->slot, + check->function); + if (*best == NULL) + return -1; + } + } + } + + return 0; } -static pciDevice * -pciGetParentDevice(pciDevice *dev) +static int +pciGetParentDevice(pciDevice *dev, pciDevice **parent) { - pciDevice *parent = NULL; - pciIterDevices(pciIsParent, dev, &parent, NULL); - return parent; + pciDevice *best = NULL; + int ret; + + *parent = NULL; + ret = pciIterDevices(pciIsParent, dev, parent, &best); + if (ret == 1) + pciFreeDevice(best); + else if (ret == 0) + *parent = best; + return ret; } /* Secondary Bus Reset is our sledgehammer - it resets all @@ -570,7 +619,8 @@ pciTrySecondaryBusReset(pciDevice *dev, } /* Find the parent bus */ - parent = pciGetParentDevice(dev); + if (pciGetParentDevice(dev, &parent) < 0) + return -1; if (!parent) { pciReportError(VIR_ERR_NO_SUPPORT, _("Failed to find parent device for %s"), @@ -1377,7 +1427,8 @@ pciDeviceIsBehindSwitchLackingACS(pciDevice *dev) { pciDevice *parent; - parent = pciGetParentDevice(dev); + if (pciGetParentDevice(dev, &parent) < 0) + return -1; if (!parent) { /* if we have no parent, and this is the root bus, ACS doesn't come * into play since devices on the root bus can't P2P without going @@ -1400,6 +1451,7 @@ pciDeviceIsBehindSwitchLackingACS(pciDevice *dev) do { pciDevice *tmp; int acs; + int ret; acs = pciDeviceDownstreamLacksACS(parent); @@ -1412,8 +1464,10 @@ pciDeviceIsBehindSwitchLackingACS(pciDevice *dev) } tmp = parent; - parent = pciGetParentDevice(parent); + ret = pciGetParentDevice(parent, &parent); pciFreeDevice(tmp); + if (ret < 0) + return -1; } while (parent); return 0; -- 1.7.1.1

3 3

[libvirt] [PATCH] Free up memballoon def.
by Chris Lalancette 02 Aug '10

02 Aug '10

Forgetting to do this was causing a memory leak. Signed-off-by: Chris Lalancette <clalance(a)redhat.com> --- src/conf/domain_conf.c | 2 ++ 1 files changed, 2 insertions(+), 0 deletions(-) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index ca4bc6e..1ddea0a 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -768,6 +768,8 @@ void virDomainDefFree(virDomainDefPtr def) virDomainWatchdogDefFree(def->watchdog); + virDomainMemballoonDefFree(def->memballoon); + virSecurityLabelDefFree(def); virCPUDefFree(def->cpu); -- 1.7.2

2 2

[libvirt] [PATCH] Don't put a semicolon on the end of a VIR_ENUM_IMPL.
by Chris Lalancette 02 Aug '10

02 Aug '10

Signed-off-by: Chris Lalancette <clalance(a)redhat.com> --- src/conf/domain_conf.c | 2 +- 1 files changed, 1 insertions(+), 1 deletions(-) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 04c417e..ca4bc6e 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -99,7 +99,7 @@ VIR_ENUM_IMPL(virDomainDeviceAddress, VIR_DOMAIN_DEVICE_ADDRESS_TYPE_LAST, "none", "pci", "drive", - "virtio-serial"); + "virtio-serial") VIR_ENUM_IMPL(virDomainDisk, VIR_DOMAIN_DISK_TYPE_LAST, "block", -- 1.7.2

2 2

[libvirt] [PATCH] Fix a bogus warning when parsing <hostdev>
by Chris Lalancette 02 Aug '10

02 Aug '10

When parsing hostdev, the following message would be emitted: 10:17:19.052: error : virDomainHostdevDefParseXML:3748 : internal error unknown node alias However, alias is appropriately parsed in virDomainDeviceInfoParseXML anyway. Disable the error message in the initial XML parsing loop. Signed-off-by: Chris Lalancette <clalance(a)redhat.com> --- src/conf/domain_conf.c | 3 +++ 1 files changed, 3 insertions(+), 0 deletions(-) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 8721dd1..04c417e 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -3723,6 +3723,9 @@ virDomainHostdevDefParseXML(const xmlNodePtr node, goto error; } } else if (xmlStrEqual(cur->name, BAD_CAST "address")) { + /* address is parsed as part of virDomainDeviceInfoParseXML */ + } else if (xmlStrEqual(cur->name, BAD_CAST "alias")) { + /* alias is parsed as part of virDomainDeviceInfoParseXML */ } else { virDomainReportError(VIR_ERR_INTERNAL_ERROR, _("unknown node %s"), cur->name); -- 1.7.2

2 2

[libvirt] What should I do when I want to update the kvm for using libvirt in thel 5.4
by 姚远 02 Aug '10

02 Aug '10

My linux is rhel 5.4. In it, the version of kvm is 83 and the qemu is 0.9.1. Virt-manager and libvirt use "/usr/libexec/qemu-kvm" to manage VMs. Now, I want to use the higher version of kvm, so I download qemu-kvm-0.12.4.tar.gz. tar xvf qemu-kvm-0.12.4.tar.gz cd qemu-kvm-0.12.4 ./configure --prefix=/usr/ make make install Then, qemu-system-x86_86 is appeared, but the executalbe file "qemu-kvm" is not generated. Virt-manager an libvirt do not use "qemu-system-x86_86", they only use "qemu-kvm". What should I do when I want to update kvm for using libvirt in thel 5.4. Maybe I can compile a new "qemu-kvm" to substitute the old one. Thank you for your reply! Best wishes!

1 0

[libvirt] default storage pool will not start
by Albert Hopkins 01 Aug '10

01 Aug '10

Long ago I had a VM that used a qcow image that was in the default directory-based storage pool. Since then the VM and the image have been deleted. But now libvirt will not start without the image file: Traceback (most recent call last): File "/usr/share/virt-manager/virtManager/host.py", line 586, in start_pool pool.start() File "/usr/share/virt-manager/virtManager/storagepool.py", line 67, in start self.pool.create(0) File "/usr/lib64/python2.6/site-packages/libvirt.py", line 1039, in create if ret == -1: raise libvirtError ('virStoragePoolCreate() failed', pool=self) libvirtError: cannot open volume '/var/lib/libvirt/images/gstable.qcow2': No such file or directory If I simply "touch" the file, then the pool starts. But I don't want it around. No other vm's reference this file. I've tried deleting and re-creating the pool. I've tried removing all the files in the pool, removing the directory, mkdir'ing the directory and then re-creating the pool, but it still will not start without that file. I've grepped /etc/libvirt for the existence of that filename but it doesn't exist. For the life of me I can't figure out where libvirt is getting this information from and why it thinks it needs this file.

2 3