- Devel - libvirt List Archives

[libvirt] [PATCH] Use printf instead of echo -e in libvirt.spec.in
by Matthias Bolte 27 May '10

27 May '10

make rpm created dummy tests containing '-e #!/bin/sh' for me. --- libvirt.spec.in | 2 +- 1 files changed, 1 insertions(+), 1 deletions(-) diff --git a/libvirt.spec.in b/libvirt.spec.in index 20ccfda..09f554e 100644 --- a/libvirt.spec.in +++ b/libvirt.spec.in @@ -658,7 +658,7 @@ cd tests for i in nodeinfotest daemon-conf seclabeltest do rm -f $i - echo -e "#!/bin/sh\nexit 0" > $i + printf "#!/bin/sh\nexit 0" > $i chmod +x $i done make check -- 1.7.0.4

2 2

[libvirt] [PATCH] build: support 'make check' in pristine tree
by Eric Blake 27 May '10

27 May '10

Otherwise, 'make check' in the python dir tries to reference a file in docs that is built by 'make' but not by 'make check'. * docs/Makefile.am (check-local): New rule. Reported by Matthias Bolte. --- Test with make && rm docs/libvirt-api.xml && make check docs/Makefile.am | 2 ++ 1 files changed, 2 insertions(+), 0 deletions(-) diff --git a/docs/Makefile.am b/docs/Makefile.am index 70b9e51..41068c6 100644 --- a/docs/Makefile.am +++ b/docs/Makefile.am @@ -143,6 +143,8 @@ html/%-%.html html/%-virterror.html %-api.xml %-refs.xml: \ $(srcdir)/../src/%.c \ $(srcdir)/../src/util/virterror.c +check-local: all + clean-local: rm -f *~ *.bak *.hierarchy *.signals *-unused.txt -- 1.7.0.1

2 2

[libvirt] [PATCH] esx: Expose host UUID in the capabilities XML
by Matthias Bolte 27 May '10

27 May '10

Parse the BIOS UUID. This information may not be available, in that case no host UUID is exposed in the capabilities XML. --- src/esx/esx_driver.c | 67 ++++++++++++++++++++++++++++++++++++++++++++++++++ 1 files changed, 67 insertions(+), 0 deletions(-) diff --git a/src/esx/esx_driver.c b/src/esx/esx_driver.c index 7c9c50e..b89de72 100644 --- a/src/esx/esx_driver.c +++ b/src/esx/esx_driver.c @@ -141,6 +141,69 @@ esxSupportsLongMode(esxPrivate *priv) +static int +esxLookupHostSystemBiosUuid(esxPrivate *priv, unsigned char *uuid) +{ + int result = -1; + esxVI_String *propertyNameList = NULL; + esxVI_ObjectContent *hostSystem = NULL; + esxVI_DynamicProperty *dynamicProperty = NULL; + + if (esxVI_EnsureSession(priv->host) < 0) { + return -1; + } + + if (esxVI_String_AppendValueToList(&propertyNameList, + "hardware.systemInfo.uuid") < 0 || + esxVI_LookupObjectContentByType(priv->host, priv->host->hostFolder, + "HostSystem", propertyNameList, + esxVI_Boolean_True, &hostSystem) < 0) { + goto cleanup; + } + + if (hostSystem == NULL) { + ESX_ERROR(VIR_ERR_INTERNAL_ERROR, "%s", + _("Could not retrieve the HostSystem object")); + goto cleanup; + } + + for (dynamicProperty = hostSystem->propSet; dynamicProperty != NULL; + dynamicProperty = dynamicProperty->_next) { + if (STREQ(dynamicProperty->name, "hardware.systemInfo.uuid")) { + if (esxVI_AnyType_ExpectType(dynamicProperty->val, + esxVI_Type_String) < 0) { + goto cleanup; + } + + if (strlen(dynamicProperty->val->string) > 0) { + if (virUUIDParse(dynamicProperty->val->string, uuid) < 0) { + ESX_ERROR(VIR_ERR_INTERNAL_ERROR, + _("Could not parse UUID from string '%s'"), + dynamicProperty->val->string); + goto cleanup; + } + } else { + /* HostSystem has an empty UUID */ + memset(uuid, 0, VIR_UUID_BUFLEN); + } + + break; + } else { + VIR_WARN("Unexpected '%s' property", dynamicProperty->name); + } + } + + result = 0; + + cleanup: + esxVI_String_Free(&propertyNameList); + esxVI_ObjectContent_Free(&hostSystem); + + return result; +} + + + static virCapsPtr esxCapsInit(esxPrivate *priv) { @@ -166,6 +229,10 @@ esxCapsInit(esxPrivate *priv) virCapabilitiesSetMacPrefix(caps, (unsigned char[]){ 0x00, 0x0c, 0x29 }); virCapabilitiesAddHostMigrateTransport(caps, "esx"); + if (esxLookupHostSystemBiosUuid(priv, caps->host.host_uuid) < 0) { + goto failure; + } + /* i686 */ guest = virCapabilitiesAddGuest(caps, "hvm", "i686", 32, NULL, NULL, 0, NULL); -- 1.7.0.4

2 2

[libvirt] [PATCH v8] add 802.1Qbh and 802.1Qbg handling
by Stefan Berger 26 May '10

26 May '10

A problem I am encountering is when trying to build after configuring with --without-macvtap. [I apologize for the recent build errors...] In that case the [dis|]associatePortProfileId functions become unused and the compiler complains. I am trying to solve this now by making them non-static functions that also later on, probably after some changes/extensions to their passed parameters, can be used in the PCI device passthrough case. I preprended 'vp' before their names, for 'virtualPort'. So the change in this version of the patch now is to not define these two functions as static. However, I have not added them to libvirt_private.syms. In case of 802.1Qbg I am sending netlink messages to (future) lldpad, another user space program. I used to use netlink multicast for doing that, but now it's unicast. I am renaming the variable multicast to nltarget_kernel and invert the logic for its testing. The two places where the variable multicast received a boolean value has now the opposite assigned to nltarget_kernel. V8: - To enable build with --without-macvtap rename the [dis|]associatePortProfileId functions, prepend 'vp' before their name and make them non-static functions. - Renaming variable multicast to nltarget_kernel and inverting the logic V7: - Addressing Jim Meyering's comments; this also touches existing code for example for correcting indentation of break statements or simplification of switch statements. Changes from v5 to v6: - Renamed occurrencvirVirtualPortProfileDef to virVirtualPortProfileParamses - 802.1Qbg part prepared for sending a RTM_SETLINK and getting processing status back plus a subsequent RTM_GETLINK to get IFLA_PORT_RESPONSE. Note: This interface for 802.1Qbg may still change Changes from v4 to v5: - [David Allan] move getPhysfn inside IFLA_VF_PORT_MAX to avoid compiler warning when latest if_link.h isn't available Changes from v3 to v4: - move from Stefan's 802.1Qb{g|h} XML v8 to v9 - move hostuuid and vf index calcs to inside doPortProfileOp8021Qbh Changes from v2 to v3: - remove debug fprintfs - use virGetHostUUID (thanks Stefan!) - fix compile issue when latest if_link.h isn't available - change poll timeout to 10s, at 1/8 intervals - if polling times out, log msg and return -ETIMEDOUT Changes from v1 to v2: - Add Stefan's code for getPortProfileStatus - Poll for up to 2 secs for port-profile status, at 1/8 sec intervals: - if status indicates error, abort openMacvtapTap - if status indicates success, exit polling - if status is "in-progress" after 2 secs of polling, exit polling loop silently, without error My patch finishes out the 802.1Qbh parts, which Stefan had mostly complete. I've tested using the recent kernel updates for VF_PORT netlink msgs and enic for Cisco's 10G Ethernet NIC. I tested many VMs, each with several direct interfaces, each configured with a port-profile per the XML. VM-to-VM, and VM-to-external work as expected. VM-to-VM on same host (using same NIC) works same as VM-to-VM where VMs are on diff hosts. I'm able to change settings on the port-profile while the VM is running to change the virtual port behaviour. For example, adjusting a QoS setting like rate limit. All VMs with interfaces using that port-profile immediatly see the effect of the change to the port-profile. I don't have a SR-IOV device to test so source dev is a non-SR-IOV device, but most of the code paths include support for specifing the source dev and VF index. We'll need to complete this by discovering the PF given the VF linkdev. Once we have the PF, we'll also have the VF index. All this info- mation is available from sysfs. Signed-off-by: Scott Feldman <scofeldm(a)cisco.com> Signed-off-by: Stefan Berger <stefanb(a)us.ibm.com> --- configure.ac | 16 src/qemu/qemu_conf.c | 2 src/qemu/qemu_driver.c | 4 src/util/macvtap.c | 833 +++++++++++++++++++++++++++++++++++++++++++++---- src/util/macvtap.h | 1 5 files changed, 785 insertions(+), 71 deletions(-) Index: libvirt-acl/configure.ac =================================================================== --- libvirt-acl.orig/configure.ac +++ libvirt-acl/configure.ac @@ -2005,13 +2005,26 @@ if test "$with_macvtap" != "no" ; then fi AM_CONDITIONAL([WITH_MACVTAP], [test "$with_macvtap" = "yes"]) +AC_TRY_COMPILE([ #include <sys/socket.h> + #include <linux/rtnetlink.h> ], + [ int x = IFLA_PORT_MAX; ], + [ with_virtualport=yes ], + [ with_virtualport=no ]) +if test "$with_virtualport" = "yes"; then + val=1 +else + val=0 +fi +AC_DEFINE_UNQUOTED([WITH_VIRTUALPORT], $val, [whether vsi vepa support is enabled]) +AM_CONDITIONAL([WITH_VIRTUALPORT], [test "$with_virtualport" = "yes"]) + dnl netlink library LIBNL_CFLAGS="" LIBNL_LIBS="" -if test "$with_macvtap" = "yes"; then +if test "$with_macvtap" = "yes" || test "$with_virtualport" = "yes"; then PKG_CHECK_MODULES([LIBNL], [libnl-1 >= $LIBNL_REQUIRED], [ ], [ AC_MSG_ERROR([libnl >= $LIBNL_REQUIRED is required for macvtap support]) @@ -2084,6 +2097,7 @@ AC_MSG_NOTICE([ Network: $with_network]) AC_MSG_NOTICE([Libvirtd: $with_libvirtd]) AC_MSG_NOTICE([ netcf: $with_netcf]) AC_MSG_NOTICE([ macvtap: $with_macvtap]) +AC_MSG_NOTICE([virtport: $with_virtualport]) AC_MSG_NOTICE([]) AC_MSG_NOTICE([Storage Drivers]) AC_MSG_NOTICE([]) Index: libvirt-acl/src/qemu/qemu_conf.c =================================================================== --- libvirt-acl.orig/src/qemu/qemu_conf.c +++ libvirt-acl/src/qemu/qemu_conf.c @@ -1509,7 +1509,7 @@ qemudPhysIfaceConnect(virConnectPtr conn if (err) { close(rc); rc = -1; - delMacvtap(net->ifname, + delMacvtap(net->ifname, net->data.direct.linkdev, &net->data.direct.virtPortProfile); } } Index: libvirt-acl/src/qemu/qemu_driver.c =================================================================== --- libvirt-acl.orig/src/qemu/qemu_driver.c +++ libvirt-acl/src/qemu/qemu_driver.c @@ -3709,7 +3709,7 @@ static void qemudShutdownVMDaemon(struct for (i = 0; i < def->nnets; i++) { virDomainNetDefPtr net = def->nets[i]; if (net->type == VIR_DOMAIN_NET_TYPE_DIRECT) - delMacvtap(net->ifname, + delMacvtap(net->ifname, net->data.direct.linkdev, &net->data.direct.virtPortProfile); } #endif @@ -8514,7 +8514,7 @@ qemudDomainDetachNetDevice(struct qemud_ #if WITH_MACVTAP if (detach->type == VIR_DOMAIN_NET_TYPE_DIRECT) - delMacvtap(detach->ifname, + delMacvtap(detach->ifname, detach->data.direct.linkdev, &detach->data.direct.virtPortProfile); #endif Index: libvirt-acl/src/util/macvtap.c =================================================================== --- libvirt-acl.orig/src/util/macvtap.c +++ libvirt-acl/src/util/macvtap.c @@ -27,7 +27,7 @@ #include <config.h> -#if WITH_MACVTAP +#if WITH_MACVTAP || WITH_VIRTUALPORT # include <stdio.h> # include <errno.h> @@ -41,6 +41,8 @@ # include <linux/rtnetlink.h> # include <linux/if_tun.h> +# include <netlink/msg.h> + # include "util.h" # include "memory.h" # include "logging.h" @@ -48,6 +50,7 @@ # include "interface.h" # include "conf/domain_conf.h" # include "virterror_internal.h" +# include "uuid.h" # define VIR_FROM_THIS VIR_FROM_NET @@ -58,14 +61,22 @@ # define MACVTAP_NAME_PREFIX "macvtap" # define MACVTAP_NAME_PATTERN "macvtap%d" +# define MICROSEC_PER_SEC (1000 * 1000) + +# define NLMSGBUF_SIZE 256 +# define RATTBUF_SIZE 64 + + +# define STATUS_POLL_TIMEOUT_USEC (10 * MICROSEC_PER_SEC) +# define STATUS_POLL_INTERVL_USEC (MICROSEC_PER_SEC / 8) + + -static int associatePortProfileId(const char *macvtap_ifname, - const virVirtualPortProfileParamsPtr virtPort, - int vf, - const unsigned char *vmuuid); -static int disassociatePortProfileId(const char *macvtap_ifname, - const virVirtualPortProfileParamsPtr virtPort); +enum virVirtualPortOp { + ASSOCIATE = 0x1, + DISASSOCIATE = 0x2, +}; static int nlOpen(void) @@ -97,7 +108,7 @@ static void nlClose(int fd) */ static int nlComm(struct nlmsghdr *nlmsg, - char **respbuf, int *respbuflen) + char **respbuf, unsigned int *respbuflen) { int rc = 0; struct sockaddr_nl nladdr = { @@ -159,6 +170,162 @@ err_exit: } +# ifdef IFLA_VF_PORT_MAX + +/** + * nlCommWaitSuccess: + * + * @nlmsg: pointer to netlink message + * @nl_grousp: the netlink multicast groups to send to + * @respbuf: pointer to pointer where response buffer will be allocated + * @respbuflen: pointer to integer holding the size of the response buffer + * on return of the function. + * @timeout_usecs: timeout in microseconds to wait for a success message + * to be returned + * + * Send the given message to the netlink multicast group and receive + * responses. Skip responses indicating an error and keep on receiving + * responses until a success response is returned. + * Returns 0 on success, -1 on error. In case of error, no response + * buffer will be returned. + */ +static int +nlCommWaitSuccess(struct nlmsghdr *nlmsg, uint32_t nl_groups, + char **respbuf, unsigned int *respbuflen, + unsigned long long timeout_usecs) +{ + int rc = 0; + struct sockaddr_nl nladdr = { + .nl_family = AF_NETLINK, + .nl_pid = getpid(), + .nl_groups = nl_groups, + }; + int rcvChunkSize = 1024; // expecting less than that + size_t rcv_offset = 0; + ssize_t nbytes; + struct timeval tv = { + .tv_sec = timeout_usecs / MICROSEC_PER_SEC, + .tv_usec = timeout_usecs % MICROSEC_PER_SEC, + }; + bool got_valid = false; + int fd = nlOpen(); + static uint32_t seq = 0x1234; + uint32_t myseq = seq++; + uint32_t mypid = getpid(); + + if (fd < 0) + return -1; + + nlmsg->nlmsg_pid = mypid; + nlmsg->nlmsg_seq = myseq; + nlmsg->nlmsg_flags |= NLM_F_ACK; + + nbytes = sendto(fd, (void *)nlmsg, nlmsg->nlmsg_len, 0, + (struct sockaddr *)&nladdr, sizeof(nladdr)); + if (nbytes < 0) { + virReportSystemError(errno, + "%s", _("cannot send to netlink socket")); + rc = -1; + goto err_exit; + } + + while (!got_valid) { + + rcv_offset = 0; + + while (1) { + int n; + fd_set rfds; + socklen_t addrlen = sizeof(nladdr); + + if (VIR_REALLOC_N(*respbuf, rcv_offset + rcvChunkSize) < 0) { + virReportOOMError(); + rc = -1; + goto err_exit; + } + + FD_ZERO(&rfds); + FD_SET(fd, &rfds); + + n = select(fd + 1, &rfds, NULL, NULL, &tv); + if (n <= 0) { + if (n < 0) + virReportSystemError(errno, "%s", + _("error in select call")); + if (n == 0) + virReportSystemError(ETIMEDOUT, "%s", + _("no valid netlink response was received")); + rc = -1; + goto err_exit; + } + + nbytes = recvfrom(fd, &((*respbuf)[rcv_offset]), rcvChunkSize, 0, + (struct sockaddr *)&nladdr, &addrlen); + if (nbytes < 0) { + if (errno == EAGAIN || errno == EINTR) + continue; + virReportSystemError(errno, "%s", + _("error receiving from netlink socket")); + rc = -1; + goto err_exit; + } + rcv_offset += nbytes; + break; + } + *respbuflen = rcv_offset; + + /* check message for error */ + if (*respbuflen > NLMSG_LENGTH(0) && *respbuf != NULL) { + struct nlmsghdr *resp = (struct nlmsghdr *)*respbuf; + struct nlmsgerr *err; + + if (resp->nlmsg_pid != mypid || + resp->nlmsg_seq != myseq) + continue; + + /* skip reflected message */ + if (resp->nlmsg_type & 0x10) + continue; + + switch (resp->nlmsg_type) { + case NLMSG_ERROR: + err = (struct nlmsgerr *)NLMSG_DATA(resp); + if (resp->nlmsg_len >= NLMSG_LENGTH(sizeof(*err))) { + if (err->error != -EOPNOTSUPP) { + /* assuming error msg from daemon */ + got_valid = true; + break; + } + } + /* whatever this is, skip it */ + VIR_FREE(*respbuf); + *respbuflen = 0; + break; + + case NLMSG_DONE: + got_valid = true; + break; + + default: + VIR_FREE(*respbuf); + *respbuflen = 0; + break; + } + } + } + +err_exit: + if (rc == -1) { + VIR_FREE(*respbuf); + *respbuflen = 0; + } + + nlClose(fd); + return rc; +} + +# endif /* IFLA_VF_PORT_MAX */ + static struct rtattr * rtattrCreate(char *buffer, int bufsize, int type, const void *data, int datalen) @@ -204,6 +371,8 @@ nlAppend(struct nlmsghdr *nlm, int totle } +# if WITH_MACVTAP + static int link_add(const char *type, const unsigned char *macaddress, int macaddrsize, @@ -213,15 +382,15 @@ link_add(const char *type, int *retry) { int rc = 0; - char nlmsgbuf[256]; + char nlmsgbuf[NLMSGBUF_SIZE]; struct nlmsghdr *nlm = (struct nlmsghdr *)nlmsgbuf, *resp; struct nlmsgerr *err; - char rtattbuf[64]; + char rtattbuf[RATTBUF_SIZE]; struct rtattr *rta, *rta1, *li; - struct ifinfomsg i = { .ifi_family = AF_UNSPEC }; + struct ifinfomsg ifinfo = { .ifi_family = AF_UNSPEC }; int ifindex; char *recvbuf = NULL; - int recvbuflen; + unsigned int recvbuflen; if (ifaceGetIndex(true, srcdev, &ifindex) != 0) return -1; @@ -232,65 +401,46 @@ link_add(const char *type, nlInit(nlm, NLM_F_REQUEST | NLM_F_CREATE | NLM_F_EXCL, RTM_NEWLINK); - if (!nlAppend(nlm, sizeof(nlmsgbuf), &i, sizeof(i))) + if (!nlAppend(nlm, sizeof(nlmsgbuf), &ifinfo, sizeof(ifinfo))) goto buffer_too_small; rta = rtattrCreate(rtattbuf, sizeof(rtattbuf), IFLA_LINK, &ifindex, sizeof(ifindex)); - if (!rta) - goto buffer_too_small; - - if (!nlAppend(nlm, sizeof(nlmsgbuf), rtattbuf, rta->rta_len)) + if (!rta || !nlAppend(nlm, sizeof(nlmsgbuf), rtattbuf, rta->rta_len)) goto buffer_too_small; rta = rtattrCreate(rtattbuf, sizeof(rtattbuf), IFLA_ADDRESS, macaddress, macaddrsize); - if (!rta) - goto buffer_too_small; - - if (!nlAppend(nlm, sizeof(nlmsgbuf), rtattbuf, rta->rta_len)) + if (!rta || !nlAppend(nlm, sizeof(nlmsgbuf), rtattbuf, rta->rta_len)) goto buffer_too_small; if (ifname) { rta = rtattrCreate(rtattbuf, sizeof(rtattbuf), IFLA_IFNAME, ifname, strlen(ifname) + 1); - if (!rta) - goto buffer_too_small; - - if (!nlAppend(nlm, sizeof(nlmsgbuf), rtattbuf, rta->rta_len)) + if (!rta || !nlAppend(nlm, sizeof(nlmsgbuf), rtattbuf, rta->rta_len)) goto buffer_too_small; } rta = rtattrCreate(rtattbuf, sizeof(rtattbuf), IFLA_LINKINFO, NULL, 0); - if (!rta) - goto buffer_too_small; - - if (!(li = nlAppend(nlm, sizeof(nlmsgbuf), rtattbuf, rta->rta_len))) + if (!rta || + !(li = nlAppend(nlm, sizeof(nlmsgbuf), rtattbuf, rta->rta_len))) goto buffer_too_small; rta = rtattrCreate(rtattbuf, sizeof(rtattbuf), IFLA_INFO_KIND, type, strlen(type)); - if (!rta) - goto buffer_too_small; - - if (!nlAppend(nlm, sizeof(nlmsgbuf), rtattbuf, rta->rta_len)) + if (!rta || !nlAppend(nlm, sizeof(nlmsgbuf), rtattbuf, rta->rta_len)) goto buffer_too_small; if (macvlan_mode > 0) { rta = rtattrCreate(rtattbuf, sizeof(rtattbuf), IFLA_INFO_DATA, NULL, 0); - if (!rta) - goto buffer_too_small; - - if (!(rta1 = nlAppend(nlm, sizeof(nlmsgbuf), rtattbuf, rta->rta_len))) + if (!rta || + !(rta1 = nlAppend(nlm, sizeof(nlmsgbuf), rtattbuf, rta->rta_len))) goto buffer_too_small; rta = rtattrCreate(rtattbuf, sizeof(rtattbuf), IFLA_MACVLAN_MODE, &macvlan_mode, sizeof(macvlan_mode)); - if (!rta) - goto buffer_too_small; - - if (!nlAppend(nlm, sizeof(nlmsgbuf), rtattbuf, rta->rta_len)) + if (!rta || !nlAppend(nlm, sizeof(nlmsgbuf), rtattbuf, rta->rta_len)) goto buffer_too_small; rta1->rta_len = (char *)nlm + nlm->nlmsg_len - (char *)rta1; @@ -312,15 +462,15 @@ link_add(const char *type, if (resp->nlmsg_len < NLMSG_LENGTH(sizeof(*err))) goto malformed_resp; - switch (-err->error) { + switch (err->error) { case 0: - break; + break; - case EEXIST: + case -EEXIST: *retry = 1; rc = -1; - break; + break; default: virReportSystemError(-err->error, @@ -328,10 +478,10 @@ link_add(const char *type, type); rc = -1; } - break; + break; case NLMSG_DONE: - break; + break; default: goto malformed_resp; @@ -358,14 +508,14 @@ static int link_del(const char *name) { int rc = 0; - char nlmsgbuf[256]; + char nlmsgbuf[NLMSGBUF_SIZE]; struct nlmsghdr *nlm = (struct nlmsghdr *)nlmsgbuf, *resp; struct nlmsgerr *err; - char rtattbuf[64]; + char rtattbuf[RATTBUF_SIZE]; struct rtattr *rta; struct ifinfomsg ifinfo = { .ifi_family = AF_UNSPEC }; char *recvbuf = NULL; - int recvbuflen; + unsigned int recvbuflen; memset(&nlmsgbuf, 0, sizeof(nlmsgbuf)); @@ -376,10 +526,7 @@ link_del(const char *name) rta = rtattrCreate(rtattbuf, sizeof(rtattbuf), IFLA_IFNAME, name, strlen(name)+1); - if (!rta) - goto buffer_too_small; - - if (!nlAppend(nlm, sizeof(nlmsgbuf), rtattbuf, rta->rta_len)) + if (!rta || !nlAppend(nlm, sizeof(nlmsgbuf), rtattbuf, rta->rta_len)) goto buffer_too_small; if (nlComm(nlm, &recvbuf, &recvbuflen) < 0) @@ -396,20 +543,16 @@ link_del(const char *name) if (resp->nlmsg_len < NLMSG_LENGTH(sizeof(*err))) goto malformed_resp; - switch (-err->error) { - case 0: - break; - - default: + if (err->error) { virReportSystemError(-err->error, _("error destroying %s interface"), name); rc = -1; } - break; + break; case NLMSG_DONE: - break; + break; default: goto malformed_resp; @@ -509,11 +652,9 @@ macvtapModeFromInt(enum virDomainNetdevM switch (mode) { case VIR_DOMAIN_NETDEV_MACVTAP_MODE_PRIVATE: return MACVLAN_MODE_PRIVATE; - break; case VIR_DOMAIN_NETDEV_MACVTAP_MODE_BRIDGE: return MACVLAN_MODE_BRIDGE; - break; case VIR_DOMAIN_NETDEV_MACVTAP_MODE_VEPA: default: @@ -654,10 +795,10 @@ create_name: cr_ifname = ifname; } - if (associatePortProfileId(cr_ifname, - virtPortProfile, - -1, - vmuuid) != 0) { + if (vpAssociatePortProfileId(cr_ifname, + linkdev, + virtPortProfile, + vmuuid) != 0) { rc = -1; goto link_del_exit; } @@ -688,8 +829,9 @@ create_name: return rc; disassociate_exit: - disassociatePortProfileId(cr_ifname, - virtPortProfile); + vpDisassociatePortProfileId(cr_ifname, + linkdev, + virtPortProfile); link_del_exit: link_del(cr_ifname); @@ -701,6 +843,7 @@ link_del_exit: /** * delMacvtap: * @ifname : The name of the macvtap interface + * @linkdev: The interface name of the NIC to connect to the external bridge * @virtPortProfile: pointer to object holding the virtual port profile data * * Delete an interface given its name. Disassociate @@ -709,22 +852,564 @@ link_del_exit: */ void delMacvtap(const char *ifname, + const char *linkdev, virVirtualPortProfileParamsPtr virtPortProfile) { if (ifname) { - disassociatePortProfileId(ifname, - virtPortProfile); + vpDisassociatePortProfileId(ifname, + linkdev, + virtPortProfile); link_del(ifname); } } +# endif /* WITH_MACVTAP */ + +# ifdef IFLA_PORT_MAX + +static struct nla_policy ifla_policy[IFLA_MAX + 1] = +{ + [IFLA_VF_PORTS] = { .type = NLA_NESTED }, +}; + +static struct nla_policy ifla_vf_ports_policy[IFLA_VF_PORT_MAX + 1] = +{ + [IFLA_VF_PORT] = { .type = NLA_NESTED }, +}; + +static struct nla_policy ifla_port_policy[IFLA_PORT_MAX + 1] = +{ + [IFLA_PORT_RESPONSE] = { .type = NLA_U16 }, +}; + + +static int +link_dump(bool nltarget_kernel, int ifindex, + struct nlattr **tb, char **recvbuf) +{ + int rc = 0; + char nlmsgbuf[NLMSGBUF_SIZE] = { 0, }; + struct nlmsghdr *nlm = (struct nlmsghdr *)nlmsgbuf, *resp; + struct nlmsgerr *err; + struct ifinfomsg ifinfo = { + .ifi_family = AF_UNSPEC, + .ifi_index = ifindex + }; + unsigned int recvbuflen; + + *recvbuf = NULL; + + nlInit(nlm, NLM_F_REQUEST, RTM_GETLINK); + + if (!nlAppend(nlm, sizeof(nlmsgbuf), &ifinfo, sizeof(ifinfo))) + goto buffer_too_small; + + if (nltarget_kernel) { + if (nlComm(nlm, recvbuf, &recvbuflen) < 0) + return -1; + } else { + if (nlCommWaitSuccess(nlm, RTMGRP_LINK, recvbuf, &recvbuflen, + 5 * MICROSEC_PER_SEC) < 0) + return -1; + } + + if (recvbuflen < NLMSG_LENGTH(0) || *recvbuf == NULL) + goto malformed_resp; + + resp = (struct nlmsghdr *)*recvbuf; + + switch (resp->nlmsg_type) { + case NLMSG_ERROR: + err = (struct nlmsgerr *)NLMSG_DATA(resp); + if (resp->nlmsg_len < NLMSG_LENGTH(sizeof(*err))) + goto malformed_resp; + + if (err->error) { + virReportSystemError(-err->error, + _("error dumping %d interface"), + ifindex); + rc = -1; + } + break; + + case GENL_ID_CTRL: + case NLMSG_DONE: + if (nlmsg_parse(resp, sizeof(struct ifinfomsg), + tb, IFLA_MAX, ifla_policy)) { + goto malformed_resp; + } + break; + + default: + goto malformed_resp; + } + + if (rc != 0) + VIR_FREE(*recvbuf); + + return rc; + +malformed_resp: + macvtapError(VIR_ERR_INTERNAL_ERROR, "%s", + _("malformed netlink response message")); + VIR_FREE(*recvbuf); + return -1; + +buffer_too_small: + macvtapError(VIR_ERR_INTERNAL_ERROR, "%s", + _("internal buffer is too small")); + return -1; +} + + +static int +getPortProfileStatus(struct nlattr **tb, int32_t vf, uint16_t *status) +{ + int rc = 1; + const char *msg = NULL; + struct nlattr *tb2[IFLA_VF_PORT_MAX + 1], + *tb3[IFLA_PORT_MAX+1]; + + if (vf == PORT_SELF_VF) { + if (tb[IFLA_PORT_SELF]) { + if (nla_parse_nested(tb3, IFLA_PORT_MAX, tb[IFLA_PORT_SELF], + ifla_port_policy)) { + msg = _("error parsing nested IFLA_VF_PORT part"); + goto err_exit; + } + } + } else { + if (tb[IFLA_VF_PORTS]) { + if (nla_parse_nested(tb2, IFLA_VF_PORT_MAX, tb[IFLA_VF_PORTS], + ifla_vf_ports_policy)) { + msg = _("error parsing nested IFLA_VF_PORTS part"); + goto err_exit; + } + if (tb2[IFLA_VF_PORT]) { + if (nla_parse_nested(tb3, IFLA_PORT_MAX, tb2[IFLA_VF_PORT], + ifla_port_policy)) { + msg = _("error parsing nested IFLA_VF_PORT part"); + goto err_exit; + } + } + } + } + + if (tb3[IFLA_PORT_RESPONSE]) { + *status = *(uint16_t *)RTA_DATA(tb3[IFLA_PORT_RESPONSE]); + rc = 0; + } else { + msg = _("no IFLA_PORT_RESPONSE found in netlink message"); + goto err_exit; + } + +err_exit: + if (msg) + macvtapError(VIR_ERR_INTERNAL_ERROR, "%s", msg); + + return rc; +} + + +static int +doPortProfileOpSetLink(bool nltarget_kernel, + int ifindex, + const char *profileId, + struct ifla_port_vsi *portVsi, + const unsigned char *instanceId, + const unsigned char *hostUUID, + int32_t vf, + uint8_t op) +{ + int rc = 0; + char nlmsgbuf[NLMSGBUF_SIZE]; + struct nlmsghdr *nlm = (struct nlmsghdr *)nlmsgbuf, *resp; + struct nlmsgerr *err; + char rtattbuf[RATTBUF_SIZE]; + struct rtattr *rta, *vfports = NULL, *vfport; + struct ifinfomsg ifinfo = { + .ifi_family = AF_UNSPEC, + .ifi_index = ifindex, + }; + char *recvbuf = NULL; + unsigned int recvbuflen = 0; + + memset(&nlmsgbuf, 0, sizeof(nlmsgbuf)); + + nlInit(nlm, NLM_F_REQUEST, RTM_SETLINK); + + if (!nlAppend(nlm, sizeof(nlmsgbuf), &ifinfo, sizeof(ifinfo))) + goto buffer_too_small; + + if (vf == PORT_SELF_VF) { + rta = rtattrCreate(rtattbuf, sizeof(rtattbuf), IFLA_PORT_SELF, NULL, 0); + } else { + rta = rtattrCreate(rtattbuf, sizeof(rtattbuf), IFLA_VF_PORTS, NULL, 0); + if (!rta || + !(vfports = nlAppend(nlm, sizeof(nlmsgbuf), + rtattbuf, rta->rta_len))) + goto buffer_too_small; + + /* begin nesting vfports */ + rta = rtattrCreate(rtattbuf, sizeof(rtattbuf), IFLA_VF_PORT, NULL, 0); + } + + if (!rta || + !(vfport = nlAppend(nlm, sizeof(nlmsgbuf), rtattbuf, rta->rta_len))) + goto buffer_too_small; + + if (profileId) { + rta = rtattrCreate(rtattbuf, sizeof(rtattbuf), IFLA_PORT_PROFILE, + profileId, strlen(profileId) + 1); + if (!rta || !nlAppend(nlm, sizeof(nlmsgbuf), rtattbuf, rta->rta_len)) + goto buffer_too_small; + } + + if (portVsi) { + rta = rtattrCreate(rtattbuf, sizeof(rtattbuf), IFLA_PORT_VSI_TYPE, + portVsi, sizeof(*portVsi)); + if (!rta || !nlAppend(nlm, sizeof(nlmsgbuf), rtattbuf, rta->rta_len)) + goto buffer_too_small; + } + + if (instanceId) { + rta = rtattrCreate(rtattbuf, sizeof(rtattbuf), IFLA_PORT_INSTANCE_UUID, + instanceId, VIR_UUID_BUFLEN); + if (!rta || !nlAppend(nlm, sizeof(nlmsgbuf), rtattbuf, rta->rta_len)) + goto buffer_too_small; + } + + if (hostUUID) { + rta = rtattrCreate(rtattbuf, sizeof(rtattbuf), IFLA_PORT_HOST_UUID, + hostUUID, VIR_UUID_BUFLEN); + if (!rta || !nlAppend(nlm, sizeof(nlmsgbuf), rtattbuf, rta->rta_len)) + goto buffer_too_small; + } + + if (vf != PORT_SELF_VF) { + rta = rtattrCreate(rtattbuf, sizeof(rtattbuf), IFLA_PORT_VF, + &vf, sizeof(vf)); + if (!rta || !nlAppend(nlm, sizeof(nlmsgbuf), rtattbuf, rta->rta_len)) + goto buffer_too_small; + } + + rta = rtattrCreate(rtattbuf, sizeof(rtattbuf), IFLA_PORT_REQUEST, + &op, sizeof(op)); + if (!rta || !nlAppend(nlm, sizeof(nlmsgbuf), rtattbuf, rta->rta_len)) + goto buffer_too_small; + + /* end nesting of vport */ + vfport->rta_len = (char *)nlm + nlm->nlmsg_len - (char *)vfport; + + if (vf != PORT_SELF_VF) { + /* end nesting of vfports */ + vfports->rta_len = (char *)nlm + nlm->nlmsg_len - (char *)vfports; + } + + if (nltarget_kernel) { + if (nlComm(nlm, &recvbuf, &recvbuflen) < 0) + return -1; + } else { + if (nlCommWaitSuccess(nlm, RTMGRP_LINK, &recvbuf, &recvbuflen, + 5 * MICROSEC_PER_SEC) < 0) + return -1; + } + + if (recvbuflen < NLMSG_LENGTH(0) || recvbuf == NULL) + goto malformed_resp; + + resp = (struct nlmsghdr *)recvbuf; + + switch (resp->nlmsg_type) { + case NLMSG_ERROR: + err = (struct nlmsgerr *)NLMSG_DATA(resp); + if (resp->nlmsg_len < NLMSG_LENGTH(sizeof(*err))) + goto malformed_resp; + + if (err->error) { + virReportSystemError(-err->error, + _("error during virtual port configuration of ifindex %d"), + ifindex); + rc = -1; + } + break; + + case NLMSG_DONE: + break; + + default: + goto malformed_resp; + } + + VIR_FREE(recvbuf); + + return rc; + +malformed_resp: + macvtapError(VIR_ERR_INTERNAL_ERROR, "%s", + _("malformed netlink response message")); + VIR_FREE(recvbuf); + return -1; + +buffer_too_small: + macvtapError(VIR_ERR_INTERNAL_ERROR, "%s", + _("internal buffer is too small")); + return -1; +} + + +static int +doPortProfileOpCommon(bool nltarget_kernel, + int ifindex, + const char *profileId, + struct ifla_port_vsi *portVsi, + const unsigned char *instanceId, + const unsigned char *hostUUID, + int32_t vf, + uint8_t op) +{ + int rc; + char *recvbuf = NULL; + struct nlattr *tb[IFLA_MAX + 1]; + int repeats = STATUS_POLL_TIMEOUT_USEC / STATUS_POLL_INTERVL_USEC; + uint16_t status = 0; + + rc = doPortProfileOpSetLink(nltarget_kernel, + ifindex, + profileId, + portVsi, + instanceId, + hostUUID, + vf, + op); + + if (rc != 0) { + macvtapError(VIR_ERR_INTERNAL_ERROR, "%s", + _("sending of PortProfileRequest failed.")); + return rc; + } + + while (--repeats >= 0) { + rc = link_dump(nltarget_kernel, ifindex, tb, &recvbuf); + if (rc) + goto err_exit; + rc = getPortProfileStatus(tb, vf, &status); + if (rc == 0) { + if (status == PORT_PROFILE_RESPONSE_SUCCESS || + status == PORT_VDP_RESPONSE_SUCCESS) { + break; + } else if (status == PORT_PROFILE_RESPONSE_INPROGRESS) { + // keep trying... + } else { + virReportSystemError(EINVAL, + _("error %d during port-profile setlink on ifindex %d"), + status, ifindex); + rc = 1; + break; + } + } else + goto err_exit; + + usleep(STATUS_POLL_INTERVL_USEC); + + VIR_FREE(recvbuf); + } + + if (status == PORT_PROFILE_RESPONSE_INPROGRESS) { + macvtapError(VIR_ERR_INTERNAL_ERROR, "%s", + _("port-profile setlink timed out")); + rc = -ETIMEDOUT; + } + +err_exit: + VIR_FREE(recvbuf); + + return rc; +} + +# endif /* IFLA_PORT_MAX */ + +static int +doPortProfileOp8021Qbg(const char *ifname, + const virVirtualPortProfileParamsPtr virtPort, + enum virVirtualPortOp virtPortOp) +{ + int rc; + +# ifndef IFLA_VF_PORT_MAX + + (void)ifname; + (void)virtPort; + (void)virtPortOp; + macvtapError(VIR_ERR_INTERNAL_ERROR, "%s", + _("Kernel VF Port support was missing at compile time.")); + rc = 1; + +# else /* IFLA_VF_PORT_MAX */ + + int op = PORT_REQUEST_ASSOCIATE; + struct ifla_port_vsi portVsi = { + .vsi_mgr_id = virtPort->u.virtPort8021Qbg.managerID, + .vsi_type_version = virtPort->u.virtPort8021Qbg.typeIDVersion, + }; + bool nltarget_kernel = false; + int ifindex; + + if (ifaceGetIndex(true, ifname, &ifindex) != 0) { + rc = 1; + goto err_exit; + } + + portVsi.vsi_type_id[2] = virtPort->u.virtPort8021Qbg.typeID >> 16; + portVsi.vsi_type_id[1] = virtPort->u.virtPort8021Qbg.typeID >> 8; + portVsi.vsi_type_id[0] = virtPort->u.virtPort8021Qbg.typeID; + + switch (virtPortOp) { + case ASSOCIATE: + op = PORT_REQUEST_ASSOCIATE; + break; + case DISASSOCIATE: + op = PORT_REQUEST_DISASSOCIATE; + break; + default: + macvtapError(VIR_ERR_INTERNAL_ERROR, + _("operation type %d not supported"), op); + rc = 1; + goto err_exit; + } + + rc = doPortProfileOpCommon(nltarget_kernel, ifindex, + NULL, + &portVsi, + virtPort->u.virtPort8021Qbg.instanceID, + NULL, + PORT_SELF_VF, + op); + +err_exit: + +# endif /* IFLA_VF_PORT_MAX */ + + return rc; +} + + +# ifdef IFLA_VF_PORT_MAX +static int +getPhysfn(const char *linkdev, + int32_t *vf, + char **physfndev) +{ + int rc = 0; + bool virtfn = false; + + if (virtfn) { + + // XXX: if linkdev is SR-IOV VF, then set vf = VF index + // XXX: and set linkdev = PF device + // XXX: need to use get_physical_function_linux() or + // XXX: something like that to get PF + // XXX: device and figure out VF index + + rc = 1; + + } else { + + /* Not SR-IOV VF: physfndev is linkdev and VF index + * refers to linkdev self + */ + + *vf = PORT_SELF_VF; + *physfndev = (char *)linkdev; + } + + return rc; +} +# endif /* IFLA_VF_PORT_MAX */ + +static int +doPortProfileOp8021Qbh(const char *ifname, + const virVirtualPortProfileParamsPtr virtPort, + const unsigned char *vm_uuid, + enum virVirtualPortOp virtPortOp) +{ + int rc; + +# ifndef IFLA_VF_PORT_MAX + + (void)ifname; + (void)virtPort; + (void)vm_uuid; + (void)virtPortOp; + macvtapError(VIR_ERR_INTERNAL_ERROR, "%s", + _("Kernel VF Port support was missing at compile time.")); + rc = 1; + +# else /* IFLA_VF_PORT_MAX */ + + char *physfndev; + unsigned char hostuuid[VIR_UUID_BUFLEN]; + int32_t vf; + int op = PORT_REQUEST_ASSOCIATE; + bool nltarget_kernel = true; + int ifindex; + + rc = virGetHostUUID(hostuuid); + if (rc) + goto err_exit; + + rc = getPhysfn(ifname, &vf, &physfndev); + if (rc) + goto err_exit; + + if (ifaceGetIndex(true, physfndev, &ifindex) != 0) { + rc = 1; + goto err_exit; + } + + switch (virtPortOp) { + case ASSOCIATE: + op = PORT_REQUEST_ASSOCIATE; + break; + case DISASSOCIATE: + op = PORT_REQUEST_DISASSOCIATE; + break; + default: + macvtapError(VIR_ERR_INTERNAL_ERROR, + _("operation type %d not supported"), op); + rc = 1; + goto err_exit; + } + + rc = doPortProfileOpCommon(nltarget_kernel, ifindex, + virtPort->u.virtPort8021Qbh.profileID, + NULL, + vm_uuid, + hostuuid, + vf, + op); + + switch (virtPortOp) { + case ASSOCIATE: + ifaceUp(ifname); + break; + case DISASSOCIATE: + ifaceDown(ifname); + break; + } + +err_exit: + +# endif /* IFLA_VF_PORT_MAX */ + + return rc; +} /** - * associatePortProfile + * vpAssociatePortProfile * * @macvtap_ifname: The name of the macvtap device * @virtPort: pointer to the object holding port profile parameters - * @vf: virtual function number, -1 if to be ignored * @vmuuid : the UUID of the virtual machine * * Associate a port on a swtich with a profile. This function @@ -736,17 +1421,16 @@ delMacvtap(const char *ifname, * Returns 0 in case of success, != 0 otherwise with error * having been reported. */ -static int -associatePortProfileId(const char *macvtap_ifname, - const virVirtualPortProfileParamsPtr virtPort, - int vf, - const unsigned char *vmuuid) +int +vpAssociatePortProfileId(const char *macvtap_ifname, + const char *linkdev, + const virVirtualPortProfileParamsPtr virtPort, + const unsigned char *vmuuid) { int rc = 0; + VIR_DEBUG("Associating port profile '%p' on link device '%s'", virtPort, macvtap_ifname); - (void)vf; - (void)vmuuid; switch (virtPort->virtPortType) { case VIR_VIRTUALPORT_NONE: @@ -754,11 +1438,14 @@ associatePortProfileId(const char *macvt break; case VIR_VIRTUALPORT_8021QBG: - + rc = doPortProfileOp8021Qbg(macvtap_ifname, virtPort, + ASSOCIATE); break; case VIR_VIRTUALPORT_8021QBH: - + rc = doPortProfileOp8021Qbh(linkdev, virtPort, + vmuuid, + ASSOCIATE); break; } @@ -767,19 +1454,22 @@ associatePortProfileId(const char *macvt /** - * disassociatePortProfile + * vpDisassociatePortProfile * * @macvtap_ifname: The name of the macvtap device + * @linkdev: The link device in case of macvtap * @virtPort: point to object holding port profile parameters * * Returns 0 in case of success, != 0 otherwise with error * having been reported. */ -static int -disassociatePortProfileId(const char *macvtap_ifname, - const virVirtualPortProfileParamsPtr virtPort) +int +vpDisassociatePortProfileId(const char *macvtap_ifname, + const char *linkdev, + const virVirtualPortProfileParamsPtr virtPort) { int rc = 0; + VIR_DEBUG("Disassociating port profile id '%p' on link device '%s' ", virtPort, macvtap_ifname); @@ -789,15 +1479,18 @@ disassociatePortProfileId(const char *ma break; case VIR_VIRTUALPORT_8021QBG: - + rc = doPortProfileOp8021Qbg(macvtap_ifname, virtPort, + DISASSOCIATE); break; case VIR_VIRTUALPORT_8021QBH: - + rc = doPortProfileOp8021Qbh(linkdev, virtPort, + NULL, + DISASSOCIATE); break; } return rc; } -#endif /* WITH_MACVTAP */ +#endif /* WITH_MACVTAP || WITH_VIRTUALPORT */ Index: libvirt-acl/src/util/macvtap.h =================================================================== --- libvirt-acl.orig/src/util/macvtap.h +++ libvirt-acl/src/util/macvtap.h @@ -72,6 +72,7 @@ int openMacvtapTap(const char *ifname, char **res_ifname); void delMacvtap(const char *ifname, + const char *linkdev, virVirtualPortProfileParamsPtr virtPortProfile); # endif /* WITH_MACVTAP */ @@ -80,6 +81,14 @@ void delMacvtap(const char *ifname, # define MACVTAP_MODE_VEPA_STR "vepa" # define MACVTAP_MODE_BRIDGE_STR "bridge" +int vpAssociatePortProfileId(const char *macvtap_ifname, + const char *linkdev, + const virVirtualPortProfileParamsPtr virtPort, + const unsigned char *vmuuid); + +int vpDisassociatePortProfileId(const char *macvtap_ifname, + const char *linkdev, + const virVirtualPortProfileParamsPtr virtPort); VIR_ENUM_DECL(virVirtualPort)

2 3

[libvirt] Just pushed a simple doc patch
by Cole Robinson 26 May '10

26 May '10

With my recent addition of vnc_allow_host_audio to qemu.conf, the user facing comment in /etc/sysconfig/libvirtd wasn't accurate. I've pushed this small fix: daemon: sysconf: Update comment about VNC audio Signed-off-by: Cole Robinson <crobinso(a)redhat.com> --- daemon/libvirtd.sysconf | 3 ++- 1 files changed, 2 insertions(+), 1 deletions(-) diff --git a/daemon/libvirtd.sysconf b/daemon/libvirtd.sysconf index 28080a0..b730c5e 100644 --- a/daemon/libvirtd.sysconf +++ b/daemon/libvirtd.sysconf @@ -11,7 +11,8 @@ # Override the QEMU/SDL default audio driver probing when # starting virtual machines using SDL graphics # -# NB these have no effect for VMs using VNC +# NB these have no effect for VMs using VNC, unless vnc_allow_host_audio +# is enabled in /etc/libvirt/qemu.conf #QEMU_AUDIO_DRV=sdl # #SDL_AUDIODRIVER=pulse -- 1.6.6.1

1 0

[libvirt] [PATCH 0/3 RFC] Demonstrating a new API for spawning processes
by Daniel P. Berrange 26 May '10

26 May '10

We have had great success with our APIs for memory allocation and string buffer management, removing most of the potential for incorrect API usage, thus avoiding many common errors. It is time todo the same for command execution. This patch series is a proof of concept for a set of APIs I've been thinking about for many months. They are intended to replace all current usage of virExec and virRun with a more flexible and less error prone API. In particular they improve of OOM handling, prevent more memory leaks & simplify alot of code that used horrible macros Patch 1 contains the actual implementation and comprehensive test suite. Patch 2 contains HTML docs. Patch 3 ports two current methods to the new APIs to demonstrate the improved code clarity. What follows is a plain text rendering of the HTML docs from the second patch describing these APIs. Spawning processes / commands from libvirt drivers This page describes the usage of libvirt APIs for spawning processes / commands from libvirt drivers. All code is required to use these APIs Problems with standard POSIX APIs The POSIX specification includes a number of APIs for spawning processes / commands, but they suffer from a number of flaws * fork+exec: The lowest & most flexible level, but very hard to use correctly / safely. It is easy to leak file descriptors, have unexpected signal handler behaviour and not handle edge cases * system: Convenient if you don't care about capturing command output, but has the serious downside that the command string is interpreted by the shell. This makes it very dangerous to use, because improperly validated user input can lead to exploits via shell meta characters. * posix_spawn: A half-way house between simplicity of system() and the flexibility of fork+exec. It does not allow for a couple of important features though, such as running a hook between the fork+exec stage, or closing all open file descriptors. Due to the problems mentioned with each of these, libvirt driver code must not use any of the above APIs. Historically libvirt provided a higher level API known as virExec. This was wrapper around fork+exec, in a similar style to posix_spawn, but with a few more features. This wrapper still suffered from a number of problems. Handling command cleanup via waitpid() is overly complex & error prone for most usage. Building up the argv[] + env[] string arrays is quite cumbersome and error prone, particularly wrt memory leak / OOM handling. The libvirt command execution API There is now a high level API that provides a safe and flexible way to spawn commands, which prevents the most common errors & is easy to code against. This code is provided in the src/util/command.h header which can be imported using #include "command.h" Defining commands in libvirt The first step is to declare what command is to be executed. The command name can be either a fully qualified path, or a bare command name. In the latter case it will be resolved wrt the $PATH environment variable. virCommandPtr cmd = virCommandNew("/usr/bin/dnsmasq"); There is no need to check for allocation failure after virCommandNew. This will be detected and reported at a later time. Adding arguments to the command There are a number of APIs for adding arguments to a command. To add a direct string arg virCommandAddArg(cmd, "-strict-order"); If an argument takes an attached value of the form -arg=val, then this can be done using virCommandAddArgPair(cmd, "--conf-file", "/etc/dnsmasq.conf"); To add a entire NULL terminated array of arguments in one go const char *const args[] = { "--strict-order", "--except-interface", "lo", "--domain", "localdomain", NULL }; virCommandAddArgSet(cmd, args); This latter option can also be used at time of initial construction of the virCommandPtr object const char *const args[] = { "--strict-order", "--except-interface", "lo", "--domain", "localdomain", NULL }; virCommandPtr cmd = virCommandNewArgs(cmd, args); Setting up the environment By default a command will inherit all environment variables from the current process. Generally this is not desirable and a customized environment will be more suitable. Any customization done via the following APIs will prevent inheritance of any existing environment variables unless explicitly allowed. The first step is usually to pass through a small number of variables from the current process. virCommandAddEnvPassCommon(cmd); This has now set up a clean environment for the child, passing through PATH, LD_PRELOAD, LD_LIBRARY_PATH, HOME, USER, LOGNAME and TMPDIR. Furthermore it will explicitly set LC_ALL=C to avoid unexpected localization of command output. Further variables can be passed through from parent explicitly: virCommandAddEnvPass(cmd, "DISPLAY"); virCommandAddEnvPass(cmd, "XAUTHORITY"); To define an environment variable in the child with an separate key / value: virCommandAddEnvPair(cmd, "TERM", "xterm"); If the key/value pair is pre-formatted in the right format, it can be set directly virCommandAddEnvString(cmd, "TERM=xterm"); Miscellaneous other options Normally the spawned command will retain the current process as its parent. If the current process dies, the child will then (usually) be terminated too. If this cleanup is not desired, then the command should be marked as daemonized: virCommandDaemonize(cmd); When daemonizing a command, the PID visible from the caller will be that of the intermediate process, not the actual damonized command. If the PID of the real command is required then a pidfile can be requested virCommandSetPidFile(cmd, "/var/run/dnsmasq.pid"); This PID file is guaranteed to be written before the intermediate process exits. Reducing command privileges Normally a command will inherit all privileges of the current process. To restrict what a command can do, it is possible to request that all its capabilities are cleared. With this done it will only be able to access resources for which it has explicit DAC permissions virCommandClearCaps(cmd); Managing file handles To prevent unintended resource leaks to child processes, all open file handles will be closed in the child, and its stdin/out/err all connected to /dev/null. It is possible to allow an open file handle to be passed into the child: virCommandPreserveFD(cmd, 5); With this file descriptor 5 in the current process remains open as file descriptor 5 in the child. For stdin/out/err it is usually neccessary to map a file handle. To attach file descriptor 7 in the current process to stdin in the child: virCommandSetInputFD(cmd, 7); Equivalently to redirect stdout or stderr in the child, pass in a pointer to the desired handle int outfd = open("out.log", "w+"); int errfd = open("err.log", "w+"); virCommandSetOutputFD(cmd, &outfd); virCommandSetErrorFD(cmd, &errfd); Alternatively it is possible to request that a pipe be created to fetch stdout/err in the parent, by initializing the FD to -1. int outfd = -1; int errfd = -1 virCommandSetOutputFD(cmd, &outfd); virCommandSetErrorFD(cmd, &errfd); Once the command is running, outfd and errfd will be initialized with valid file handles that can be read from. Feeding & capturing strings to/from the child Often dealing with file handles for stdin/out/err is unneccessarily complex. It is possible to specify a string buffer to act as the data source for the child's stdin const char *input = "Hello World\n"; virCommandSetInputBuffer(cmd, input); Similarly it is possible to request that the child's stdout/err be redirected into a string buffer char *output = NULL, *errors = NULL; virCommandSetOutputBuffer(cmd, &output); virCommandSetErrorBuffer(cmd, &errors); Once the command has finished executing, these buffers will contain the output. It is the callers responsibility to free these buffers. Running commands synchronously For most commands, the desired behaviour is to spawn the command, wait for it to complete & exit and then check that its exit status is zero. if (virCommandRun(cmd, NULL) < 0) return -1; Note: if the command has been daemonized this will only block & wait for the intermediate process, not the real command. virCommandRun will report on any errors that have occured upon this point with all previous API calls. If the command fails to run, or exits with non-zero status an error will be reported via normal libvirt error infrastructure. If a non-zero exit status can represent a success condition, it is possible to request the exit status and perform that check manually instead of letting virCommandRun raise the error int status; if (virCommandRun(cmd, &status) < 0) return -1; if (WEXITSTATUS(status) ...) { ...do stuff... } Running commands asynchronously In certain complex scenarios, particularly special I/O handling is required for the child's stdin/err/out it will be neccessary to run the command asynchronously and wait for completion separately. pid_t pid; if (virCommandRunAsync(cmd, &pid) < 0) return -1; ... do something while pid is running ... int status; if (virCommandWait(cmd, &status) < 0) return -1; if (WEXITSTATUS(status)...) { ..do stuff.. } As with virCommandRun, the status arg for virCommandWait can be omitted, in which case it will validate that exit status is zero and raise an error if not. Releasing resources Once the command has been executed, or if execution has been abandoned, it is neccessary to release resources associated with the virCommandPtr object. This is done with: virCommandFree(cmd); There is no need to check if cmd is NULL before calling virCommandFree. This scenario is handled automatically. If the command is still running, it will be forcably killed and cleaned up (via waitpid). Complete examples This shows a complete example usage of the APIs roughly using the libvirt source src/util/hooks.c int runhook(const char *drvstr, const char *id, const char *opstr, const char *subopstr, const char *extra) { int ret; char *path; virCommandPtr cmd; ret = virBuildPath(&path, LIBVIRT_HOOK_DIR, drvstr); if ((ret < 0) || (path == NULL)) { virHookReportError(VIR_ERR_INTERNAL_ERROR, _("Failed to build path for %s hook"), drvstr); return -1; } cmd = virCommandNew(path); VIR_FREE(path); virCommandAddEnvPassCommon(cmd); virCommandAddArg(cmd, id); virCommandAddArg(cmd, opstr); virCommandAddArg(cmd, subopstr); virCommandAddArg(cmd, extra); virCommandSetInputBuffer(cmd, input); ret = virCommandRun(cmd, NULL); virCommandFree(cmd); return ret; In this example, the command is being run synchronously. A pre-formatted string is being fed to the command as its stdin. The command takes four arguments, and has a minimal set of environment variables passed down. In this example, the code does not require any error checking. All errors are reported by the virCommandRun method, and the exit status from this is returned to the caller to handle as desired.

3 13

[libvirt] [PATCH] build: fix HTML errors in nwfilter docs
by Eric Blake 26 May '10

26 May '10

A build on Ubuntu reported: || Generating formatnwfilter.html.tmp /dados/develop/libvirt/docs/formatnwfilter.html.in|390| HTML parser error : Unexpected end tag : p || </p> || ^ /dados/develop/libvirt/docs/formatnwfilter.html.in|705| HTML parser error : Unexpected end tag : code || <td>End of range of valid source ports</code></td> || ^ /dados/develop/libvirt/docs/formatnwfilter.html.in|710| HTML parser error : Unexpected end tag : code || <td>Start of range of valid destination ports</code></td> || ^ * docs/formatnwfilter.html.in: Fix invalid HTML constructs. Reported by Eduardo Otubo. --- Don't know why a build on Fedora didn't catch these, so I won't push until I get an ack that this fixes things on Ubuntu. docs/formatnwfilter.html.in | 11 ++++++----- 1 files changed, 6 insertions(+), 5 deletions(-) diff --git a/docs/formatnwfilter.html.in b/docs/formatnwfilter.html.in index f03e9ce..586a4fb 100644 --- a/docs/formatnwfilter.html.in +++ b/docs/formatnwfilter.html.in @@ -384,10 +384,11 @@ </table> <p> Valid Strings for <code>protocolid</code> are: arp, rarp, ipv4, ipv6 - <br><br> - Example: <pre><mac match='no' srcmacaddr='$MAC'/></pre> - <br><br> </p> +<pre> +[...] +<mac match='no' srcmacaddr='$MAC'/> +</pre> <h5><a name="nwfelemsRulesProtoARP">ARP/RARP</a></h5> <p> @@ -702,12 +703,12 @@ <tr> <td>srcportend</td> <td>UINT16</td> - <td>End of range of valid source ports</code></td> + <td>End of range of valid source ports</td> </tr> <tr> <td>dstportstart</td> <td>UINT16</td> - <td>Start of range of valid destination ports</code></td> + <td>Start of range of valid destination ports</td> </tr> <tr> <td>dstportend</td> -- 1.7.0.1

3 4

[libvirt] [PATCH] Clear/invalidate partially parsed UUIDs
by Stefan Berger 26 May '10

26 May '10

Getting paranoid now... Clear possibly partially parsed UUIDs. Question is whether to do this in the parsing function itself. Signed-off-by: Stefan Berger <stefanb(a)us.ibm.com> --- src/util/uuid.c | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) Index: libvirt-acl/src/util/uuid.c =================================================================== --- libvirt-acl.orig/src/util/uuid.c +++ libvirt-acl/src/util/uuid.c @@ -289,14 +289,19 @@ virSetHostUUIDStr(const char *uuid) if (!getDMISystemUUID(dmiuuid, sizeof(dmiuuid))) { if (!virUUIDParse(dmiuuid, host_uuid)) return 0; + // clear partially parsed UUID + memset(host_uuid, 0x0, sizeof(host_uuid)); } if (!virUUIDIsValid(host_uuid)) return virUUIDGenerate(host_uuid); } else { rc = virUUIDParse(uuid, host_uuid); - if (rc) + if (rc) { + // clear partially parsed UUID + memset(host_uuid, 0x0, sizeof(host_uuid)); return rc; + } if (!virUUIDIsValid(host_uuid)) return EINVAL; }

2 1

[libvirt] [PATCH] Fix integer overflow in hotplug methods
by Daniel P. Berrange 26 May '10

26 May '10

The hotplug methods still had the qemuCmdFlags variable declared as an int, instead of unsigned long long. This caused flag checks to be incorrect for flags > 31 * src/qemu/qemu_driver.c: Fix integer overflow in hotplug --- src/qemu/qemu_driver.c | 16 ++++++++-------- 1 files changed, 8 insertions(+), 8 deletions(-) diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 64dbf4a..ad99d59 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -7019,7 +7019,7 @@ error: static int qemudDomainAttachPciDiskDevice(struct qemud_driver *driver, virDomainObjPtr vm, virDomainDiskDefPtr disk, - int qemuCmdFlags) + unsigned long long qemuCmdFlags) { int i, ret; const char* type = virDomainDiskBusTypeToString(disk->bus); @@ -7114,7 +7114,7 @@ error: static int qemudDomainAttachPciControllerDevice(struct qemud_driver *driver, virDomainObjPtr vm, virDomainControllerDefPtr controller, - int qemuCmdFlags) + unsigned long long qemuCmdFlags) { int i; int ret = -1; @@ -7180,7 +7180,7 @@ static virDomainControllerDefPtr qemuDomainFindOrCreateSCSIDiskController(struct qemud_driver *driver, virDomainObjPtr vm, int controller, - int qemuCmdFlags) + unsigned long long qemuCmdFlags) { int i; virDomainControllerDefPtr cont; @@ -7225,7 +7225,7 @@ qemuDomainFindOrCreateSCSIDiskController(struct qemud_driver *driver, static int qemudDomainAttachSCSIDisk(struct qemud_driver *driver, virDomainObjPtr vm, virDomainDiskDefPtr disk, - int qemuCmdFlags) + unsigned long long qemuCmdFlags) { int i; qemuDomainObjPrivatePtr priv = vm->privateData; @@ -7341,7 +7341,7 @@ error: static int qemudDomainAttachUsbMassstorageDevice(struct qemud_driver *driver, virDomainObjPtr vm, virDomainDiskDefPtr disk, - int qemuCmdFlags) + unsigned long long qemuCmdFlags) { qemuDomainObjPrivatePtr priv = vm->privateData; int i, ret; @@ -7652,7 +7652,7 @@ no_memory: static int qemudDomainAttachHostPciDevice(struct qemud_driver *driver, virDomainObjPtr vm, virDomainHostdevDefPtr hostdev, - int qemuCmdFlags) + unsigned long long qemuCmdFlags) { qemuDomainObjPrivatePtr priv = vm->privateData; pciDevice *pci; @@ -7764,7 +7764,7 @@ error: static int qemudDomainAttachHostUsbDevice(struct qemud_driver *driver, virDomainObjPtr vm, virDomainHostdevDefPtr hostdev, - int qemuCmdFlags) + unsigned long long qemuCmdFlags) { int ret; qemuDomainObjPrivatePtr priv = vm->privateData; @@ -7808,7 +7808,7 @@ error: static int qemudDomainAttachHostDevice(struct qemud_driver *driver, virDomainObjPtr vm, virDomainHostdevDefPtr hostdev, - int qemuCmdFlags) + unsigned long long qemuCmdFlags) { if (hostdev->mode != VIR_DOMAIN_HOSTDEV_MODE_SUBSYS) { qemuReportError(VIR_ERR_NO_SUPPORT, -- 1.6.6.1

3 2

[libvirt] [PATCH] Fix failing virGetHostname.
by Chris Lalancette 26 May '10

26 May '10

We've been running into a lot of situations where virGetHostname() is returning "localhost", where a plain gethostname() would have returned the correct thing. This is because virGetHostname() is *always* trying to canonicalize the name returned from gethostname(), even when it doesn't have to. This patch changes virGetHostname so that if the value returned from gethostname() is already FQDN or localhost, it returns that string directly. If the value returned from gethostname() is a shortened hostname, then we try to canonicalize it. If that succeeds, we returned the canonicalized hostname. If that fails, and/or returns "localhost", then we just return the original string we got from gethostname() and hope for the best. Note that after this patch it is up to clients to check whether "localhost" is an allowed return value. The only place where it's currently not is in qemu migration. Signed-off-by: Chris Lalancette <clalance(a)redhat.com> --- src/libvirt_private.syms | 1 - src/qemu/qemu_driver.c | 15 +++++-- src/util/util.c | 94 ++++++++++++++++++++++++--------------------- src/util/util.h | 1 - 4 files changed, 60 insertions(+), 51 deletions(-) diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index 104278f..130a2a1 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -653,7 +653,6 @@ virExecDaemonize; virSetCloseExec; virSetNonBlock; virFormatMacAddr; -virGetHostnameLocalhost; virGetHostname; virParseMacAddr; virFileDeletePid; diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 0af64c7..cfa5964 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -10017,7 +10017,7 @@ qemudDomainMigratePrepare2 (virConnectPtr dconn, virDomainDefPtr def = NULL; virDomainObjPtr vm = NULL; int this_port; - char *hostname; + char *hostname = NULL; char migrateFrom [64]; const char *p; virDomainEventPtr event = NULL; @@ -10057,9 +10057,15 @@ qemudDomainMigratePrepare2 (virConnectPtr dconn, if (port == QEMUD_MIGRATION_NUM_PORTS) port = 0; /* Get hostname */ - if ((hostname = virGetHostnameLocalhost(0)) == NULL) + if ((hostname = virGetHostname(NULL)) == NULL) goto cleanup; + if (STRPREFIX(hostname, "localhost")) { + qemuReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("hostname on destination resolved to localhost, but migration requires an FQDN")); + goto cleanup; + } + /* XXX this really should have been a properly well-formed * URI, but we can't add in tcp:// now without breaking * compatability with old targets. We at least make the @@ -10067,7 +10073,6 @@ qemudDomainMigratePrepare2 (virConnectPtr dconn, */ /* Caller frees */ internalret = virAsprintf(uri_out, "tcp:%s:%d", hostname, this_port); - VIR_FREE(hostname); if (internalret < 0) { virReportOOMError(); goto cleanup; @@ -10171,10 +10176,10 @@ endjob: vm = NULL; cleanup: + VIR_FREE(hostname); virDomainDefFree(def); - if (ret != 0) { + if (ret != 0) VIR_FREE(*uri_out); - } if (vm) virDomainObjUnlock(vm); if (event) diff --git a/src/util/util.c b/src/util/util.c index e937d39..930bfac 100644 --- a/src/util/util.c +++ b/src/util/util.c @@ -2367,11 +2367,31 @@ char *virIndexToDiskName(int idx, const char *prefix) # define AI_CANONIDN 0 #endif -char *virGetHostnameLocalhost(int allow_localhost) +/* Who knew getting a hostname could be so delicate. In Linux (and Unices + * in general), many things depend on "hostname" returning a value that will + * resolve one way or another. In the modern world where networks frequently + * come and go this is often being hard-coded to resolve to "localhost". If + * it *doesn't* resolve to localhost, then we would prefer to have the FQDN. + * That leads us to 3 possibilities: + * + * 1) gethostname() returns an FQDN (not localhost) - we return the string + * as-is, it's all of the information we want + * 2) gethostname() returns "localhost" - we return localhost; doing further + * work to try to resolve it is pointless + * 3) gethostname() returns a shortened hostname - in this case, we want to + * try to resolve this to a fully-qualified name. Therefore we pass it + * to getaddrinfo(). There are two possible responses: + * a) getaddrinfo() resolves to a FQDN - return the FQDN + * b) getaddrinfo() resolves to localhost - in this case, the data we got + * from gethostname() is actually more useful than what we got from + * getaddrinfo(). Return the value from gethostname() and hope for + * the best. + */ +char *virGetHostname(virConnectPtr conn ATTRIBUTE_UNUSED) { int r; char hostname[HOST_NAME_MAX+1], *result; - struct addrinfo hints, *info, *res; + struct addrinfo hints, *info; r = gethostname (hostname, sizeof(hostname)); if (r == -1) { @@ -2381,6 +2401,21 @@ char *virGetHostnameLocalhost(int allow_localhost) } NUL_TERMINATE(hostname); + if (STRPREFIX(hostname, "localhost") || strchr(hostname, '.')) { + /* in this case, gethostname returned localhost (meaning we can't + * do any further canonicalization), or it returned an FQDN (and + * we don't need to do any further canonicalization). Return the + * string as-is; it's up to callers to check whether "localhost" + * is allowed. + */ + result = strdup(hostname); + goto check_and_return; + } + + /* otherwise, it's a shortened, non-localhost, hostname. Attempt to + * canonicalize the hostname by running it through getaddrinfo + */ + memset(&hints, 0, sizeof(hints)); hints.ai_flags = AI_CANONNAME|AI_CANONIDN; hints.ai_family = AF_UNSPEC; @@ -2395,54 +2430,25 @@ char *virGetHostnameLocalhost(int allow_localhost) /* Tell static analyzers about getaddrinfo semantics. */ sa_assert (info); - /* if we aren't allowing localhost, then we iterate through the - * list and make sure none of the IPv4 addresses are 127.0.0.1 and - * that none of the IPv6 addresses are ::1 - */ - if (!allow_localhost) { - res = info; - while (res) { - if (res->ai_family == AF_INET) { - if (htonl(((struct sockaddr_in *)res->ai_addr)->sin_addr.s_addr) == INADDR_LOOPBACK) { - virUtilError(VIR_ERR_INTERNAL_ERROR, "%s", - _("canonical hostname pointed to localhost, but this is not allowed")); - freeaddrinfo(info); - return NULL; - } - } - else if (res->ai_family == AF_INET6) { - if (IN6_IS_ADDR_LOOPBACK(&((struct sockaddr_in6 *)res->ai_addr)->sin6_addr)) { - virUtilError(VIR_ERR_INTERNAL_ERROR, "%s", - _("canonical hostname pointed to localhost, but this is not allowed")); - freeaddrinfo(info); - return NULL; - } - } - res = res->ai_next; - } - } + if (info->ai_canonname == NULL || + STRPREFIX(info->ai_canonname, "localhost")) + /* in this case, we tried to canonicalize and we ended up back with + * localhost. Ignore the canonicalized name and just return the + * original hostname + */ + result = strdup(hostname); + else + /* Caller frees this string. */ + result = strdup (info->ai_canonname); - if (info->ai_canonname == NULL) { - virUtilError(VIR_ERR_INTERNAL_ERROR, - "%s", _("could not determine canonical host name")); - freeaddrinfo(info); - return NULL; - } + freeaddrinfo(info); - /* Caller frees this string. */ - result = strdup (info->ai_canonname); - if (!result) +check_and_return: + if (result == NULL) virReportOOMError(); - - freeaddrinfo(info); return result; } -char *virGetHostname(virConnectPtr conn ATTRIBUTE_UNUSED) -{ - return virGetHostnameLocalhost(1); -} - /* send signal to a single process */ int virKillProcess(pid_t pid, int sig) { diff --git a/src/util/util.h b/src/util/util.h index 6bf6bcc..f8b64c2 100644 --- a/src/util/util.h +++ b/src/util/util.h @@ -252,7 +252,6 @@ static inline int getuid (void) { return 0; } static inline int getgid (void) { return 0; } # endif -char *virGetHostnameLocalhost(int allow_localhost); char *virGetHostname(virConnectPtr conn); int virKillProcess(pid_t pid, int sig); -- 1.6.6.1

5 18