- Devel - libvirt List Archives

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

29 May '10

This is now hopefully the final version of this patch that adds support for configuring 802.1Qbg and 802.1Qbh switches. The 802.1Qbh part has been successfully tested with real hardware. The 802.1Qbg part has only been tested with a (dummy) server that 'behaves' similarly to how we expect lldpad to 'behave'. V11: - determining pid of lldpad daemon by reading it from /var/run/libvirt.pid (hardcode as is hardcode alson in lldpad sources) - merging netlink send code for kernel target and user space target (lldpad) using one function nlComm() to send the messages - adding a select() after the sending and before the reading of the netlink response in case lldpad doesn't respond and so we don't hang - when reading the port status, in case of 802.1Qbg, no status may be received while things are 'in progress' and only at the end a status will be there. - when reading the port status, use the given instanceId and vf to pick the right IFLA_VF_PORT among those nested under IFLA_VF_PORTS. V10: - never sending nor parsing IFLA_PORT_SELF type of messages in the 802.1Qbg case - iterating over the elements in a IFLA_VF_PORTS to pick the right IFLA_VF_PORT by either IFLA_PORT_PROFILE and given profileId (802.1Qbh) or IFLA_PORT_INSTANCE_UUID and given instanceId (802.1Qbg) and reading the current status in IFLA_PORT_RESPONSE. V9: - recycling a previous patch that adds functionality to interface.c to - get the vlan identifier on an interface - get the flags of an interface and some convenience function to check whether an interface is 'up' or not (not currently used here) - adding function to determine the root physical interface of an interface. For example if a macvtap is linked to eth0.100, it will find eth0. Also adding a function that finds the vlan on the 'way to the root physical interface' - conveying the root physical interface name and index in case of 802.1Qbg - conveying mac address of macvlan device and vlan identifier in IFLA_VFINFO_LIST[ IFLA_VF_INFO[ IFLA_VF_MAC(mac), IFLA_VF_VLAN(vlan) ] ] to (future) lldpad via netlink 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 | 17 src/libvirt_private.syms | 3 src/qemu/qemu_conf.c | 2 src/qemu/qemu_driver.c | 4 src/util/interface.c | 141 ++++++ src/util/interface.h | 5 src/util/macvtap.c | 1007 ++++++++++++++++++++++++++++++++++++++++++----- src/util/macvtap.h | 10 8 files changed, 1083 insertions(+), 106 deletions(-) Index: libvirt-acl/configure.ac =================================================================== --- libvirt-acl.orig/configure.ac +++ libvirt-acl/configure.ac @@ -2005,13 +2005,27 @@ 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 +2098,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 @@ -1554,7 +1554,7 @@ qemudPhysIfaceConnect(virConnectPtr conn if (err) { close(rc); rc = -1; - delMacvtap(net->ifname, + delMacvtap(net->ifname, net->data.direct.linkdev, &net->data.direct.virtPortProfile); VIR_FREE(net->ifname); } 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); VIR_FREE(net->ifname); } @@ -8548,7 +8548,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); VIR_FREE(detach->ifname); } 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,24 @@ # 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 NETLINK_ACK_TIMEOUT_S 2 + +# 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); +#define LLDPAD_PID_FILE "/var/run/lldpad.pid" + + +enum virVirtualPortOp { + ASSOCIATE = 0x1, + DISASSOCIATE = 0x2, +}; static int nlOpen(void) @@ -90,6 +103,7 @@ static void nlClose(int fd) * @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. + * @nl_pid: the pid of the process to talk to, i.e., pid = 0 for kernel * * Send the given message to the netlink layer and receive response. * Returns 0 on success, -1 on error. In case of error, no response @@ -97,22 +111,29 @@ static void nlClose(int fd) */ static int nlComm(struct nlmsghdr *nlmsg, - char **respbuf, int *respbuflen) + char **respbuf, unsigned int *respbuflen, + int nl_pid) { int rc = 0; struct sockaddr_nl nladdr = { .nl_family = AF_NETLINK, - .nl_pid = 0, + .nl_pid = nl_pid, .nl_groups = 0, }; int rcvChunkSize = 1024; // expecting less than that int rcvoffset = 0; ssize_t nbytes; + struct timeval tv = { + .tv_sec = NETLINK_ACK_TIMEOUT_S, + }; + fd_set readfds; int fd = nlOpen(); + int n; if (fd < 0) return -1; + nlmsg->nlmsg_pid = getpid(); nlmsg->nlmsg_flags |= NLM_F_ACK; nbytes = sendto(fd, (void *)nlmsg, nlmsg->nlmsg_len, 0, @@ -124,6 +145,21 @@ int nlComm(struct nlmsghdr *nlmsg, goto err_exit; } + FD_ZERO(&readfds); + FD_SET(fd, &readfds); + + n = select(fd + 1, &readfds, 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; + } + while (1) { if (VIR_REALLOC_N(*respbuf, rcvoffset+rcvChunkSize) < 0) { virReportOOMError(); @@ -204,6 +240,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 +251,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 +270,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; @@ -298,7 +317,7 @@ link_add(const char *type, li->rta_len = (char *)nlm + nlm->nlmsg_len - (char *)li; - if (nlComm(nlm, &recvbuf, &recvbuflen) < 0) + if (nlComm(nlm, &recvbuf, &recvbuflen, 0) < 0) return -1; if (recvbuflen < NLMSG_LENGTH(0) || recvbuf == NULL) @@ -312,15 +331,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 +347,10 @@ link_add(const char *type, type); rc = -1; } - break; + break; case NLMSG_DONE: - break; + break; default: goto malformed_resp; @@ -358,14 +377,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,13 +395,10 @@ 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) + if (nlComm(nlm, &recvbuf, &recvbuflen, 0) < 0) return -1; if (recvbuflen < NLMSG_LENGTH(0) || recvbuf == NULL) @@ -396,20 +412,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 +521,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 +664,11 @@ create_name: cr_ifname = ifname; } - if (associatePortProfileId(cr_ifname, - virtPortProfile, - -1, - vmuuid) != 0) { + if (vpAssociatePortProfileId(cr_ifname, + macaddress, + linkdev, + virtPortProfile, + vmuuid) != 0) { rc = -1; goto link_del_exit; } @@ -688,8 +699,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 +713,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 +722,823 @@ 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_port_policy[IFLA_PORT_MAX + 1] = +{ + [IFLA_PORT_RESPONSE] = { .type = NLA_U16 }, +}; + + +static uint32_t +getLldpadPid(void) { + int fd; + uint32_t pid = 0; + + fd = open(LLDPAD_PID_FILE, O_RDONLY); + if (fd >= 0) { + char buffer[10]; + char *endptr; + + if (saferead(fd, buffer, sizeof(buffer)) <= sizeof(buffer)) { + unsigned int res; + + if (virStrToLong_ui(buffer, &endptr, 10, &res)) + macvtapError(VIR_ERR_INTERNAL_ERROR, "%s", + "error parsing pid of lldpad"); + else + pid = res; + } + } else { + virReportSystemError(errno, + _("Error opening file %s"), LLDPAD_PID_FILE); + } + + if (fd >= 0) + close(fd); + + return pid; +} + + +static int +link_dump(bool nltarget_kernel, const char *ifname, 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; + char rtattbuf[RATTBUF_SIZE]; + struct rtattr *rta; + struct ifinfomsg ifinfo = { + .ifi_family = AF_UNSPEC, + .ifi_index = ifindex + }; + unsigned int recvbuflen; + uint32_t pid = 0; + + *recvbuf = NULL; + + nlInit(nlm, NLM_F_REQUEST, RTM_GETLINK); + + if (!nlAppend(nlm, sizeof(nlmsgbuf), &ifinfo, sizeof(ifinfo))) + goto buffer_too_small; + + if (ifindex < 0 && ifname) { + rta = rtattrCreate(rtattbuf, sizeof(rtattbuf), IFLA_IFNAME, + ifname, strlen(ifname) + 1); + if (!rta || !nlAppend(nlm, sizeof(nlmsgbuf), rtattbuf, rta->rta_len)) + goto buffer_too_small; + } + + if (!nltarget_kernel) { + pid = getLldpadPid(); + if (pid == 0) + return -1; + } + + if (nlComm(nlm, recvbuf, &recvbuflen, pid) < 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 %s (%d) interface"), + ifname, 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; +} + + +/** + * ifaceGetNthParent + * + * @ifindex : the index of the interface or -1 if ifname is given + * @ifname : the name of the interface; ignored if ifindex is valid + * @nthParent : the nth parent interface to get + * @parent_ifindex : pointer to int + * @parent_ifname : pointer to buffer of size IFNAMSIZ + * @nth : the nth parent that is actually returned; if for example eth0.100 + * was given and the 100th parent is to be returned, then eth0 will + * most likely be returned with nth set to 1 since the chain does + * not have more interfaces + * + * Get the nth parent interface of the given interface. 0 is the interface + * itself. + * + * Return 0 on success, != 0 otherwise + */ +static int +ifaceGetNthParent(int ifindex, const char *ifname, unsigned int nthParent, + int *parent_ifindex, char *parent_ifname, + unsigned int *nth) +{ + int rc; + struct nlattr *tb[IFLA_MAX + 1] = { NULL, }; + char *recvbuf = NULL; + bool end = false; + unsigned int i = 0; + + *nth = 0; + + while (!end && i <= nthParent) { + rc = link_dump(true, ifname, ifindex, tb, &recvbuf); + if (rc) + break; + + if (tb[IFLA_IFNAME]) { + if (!virStrcpy(parent_ifname, (char*)RTA_DATA(tb[IFLA_IFNAME]), + IFNAMSIZ)) { + macvtapError(VIR_ERR_INTERNAL_ERROR, "%s", + _("buffer for root interface name is too small")); + VIR_FREE(recvbuf); + return 1; + } + *parent_ifindex = ifindex; + } + + if (tb[IFLA_LINK]) { + ifindex = *(int *)RTA_DATA(tb[IFLA_LINK]); + ifname = NULL; + } else + end = true; + + VIR_FREE(recvbuf); + + i++; + } + + if (nth) + *nth = i - 1; + + return rc; +} /** - * associatePortProfile + * getPortProfileStatus + * + * tb: top level netlink response attributes + values + * vf: The virtual function used in the request + * instanceId: instanceId of the interface (vm uuid in case of 802.1Qbh) + * is8021Qbg: whether this function is call for 8021Qbg + * status: pointer to a uint16 where the status will be written into + * + * Get the status from the IFLA_PORT_RESPONSE field; Returns 0 in + * case of success, != 0 otherwise with error having been reported + */ +static int +getPortProfileStatus(struct nlattr **tb, int32_t vf, + const unsigned char *instanceId, + bool nltarget_kernel, + bool is8021Qbg, + uint16_t *status) +{ + int rc = 1; + const char *msg = NULL; + struct nlattr *tb_port[IFLA_PORT_MAX + 1] = { NULL, }; + + if (vf == PORT_SELF_VF && nltarget_kernel) { + if (tb[IFLA_PORT_SELF]) { + if (nla_parse_nested(tb_port, IFLA_PORT_MAX, tb[IFLA_PORT_SELF], + ifla_port_policy)) { + msg = _("error parsing IFLA_VF_SELF part"); + goto err_exit; + } + } else { + msg = _("IFLA_VF_SELF is missing"); + goto err_exit; + } + } else { + if (tb[IFLA_VF_PORTS]) { + int rem; + bool found = false; + struct nlattr *tb_vf_ports = { NULL, }; + + nla_for_each_nested(tb_vf_ports, tb[IFLA_VF_PORTS], rem) { + + if (nla_type(tb_vf_ports) != IFLA_VF_PORT) { + msg = _("error while iterating over IFLA_VF_PORTS part"); + goto err_exit; + } + + if (nla_parse_nested(tb_port, IFLA_PORT_MAX, tb_vf_ports, + ifla_port_policy)) { + msg = _("error parsing IFLA_VF_PORT part"); + goto err_exit; + } + + if (instanceId && + tb_port[IFLA_PORT_INSTANCE_UUID] && + !memcmp(instanceId, + (unsigned char *) + RTA_DATA(tb_port[IFLA_PORT_INSTANCE_UUID]), + VIR_UUID_BUFLEN) && + tb_port[IFLA_PORT_VF] && + vf == *(uint32_t *)RTA_DATA(tb_port[IFLA_PORT_VF])) { + found = true; + break; + } + } + + if (!found) { + msg = _("Could not find netlink response with " + "expected parameters"); + goto err_exit; + } + } else { + msg = _("IFLA_VF_PORTS is missing"); + goto err_exit; + } + } + + if (tb_port[IFLA_PORT_RESPONSE]) { + *status = *(uint16_t *)RTA_DATA(tb_port[IFLA_PORT_RESPONSE]); + rc = 0; + } else { + if (is8021Qbg) { + /* no in-progress here; may be missing */ + *status = PORT_PROFILE_RESPONSE_INPROGRESS; + } 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, + const char *ifname, int ifindex, + const unsigned char *macaddr, + int vlanid, + 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; + uint32_t pid = 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 (ifname) { + rta = rtattrCreate(rtattbuf, sizeof(rtattbuf), IFLA_IFNAME, + ifname, strlen(ifname) + 1); + if (!rta || !nlAppend(nlm, sizeof(nlmsgbuf), rtattbuf, rta->rta_len)) + goto buffer_too_small; + } + + if (macaddr && vlanid >= 0) { + struct rtattr *vfinfolist, *vfinfo; + struct ifla_vf_mac ifla_vf_mac = { + .vf = vf, + .mac = { 0, }, + }; + struct ifla_vf_vlan ifla_vf_vlan = { + .vf = vf, + .vlan = vlanid, + .qos = 0, + }; + + memcpy(ifla_vf_mac.mac, macaddr, 6); + + rta = rtattrCreate(rtattbuf, sizeof(rtattbuf), IFLA_VFINFO_LIST, + NULL, 0); + if (!rta || + !(vfinfolist = nlAppend(nlm, sizeof(nlmsgbuf), + rtattbuf, rta->rta_len))) + goto buffer_too_small; + + rta = rtattrCreate(rtattbuf, sizeof(rtattbuf), IFLA_VF_INFO, + NULL, 0); + if (!rta || + !(vfinfo = nlAppend(nlm, sizeof(nlmsgbuf), + rtattbuf, rta->rta_len))) + goto buffer_too_small; + + rta = rtattrCreate(rtattbuf, sizeof(rtattbuf), IFLA_VF_MAC, + &ifla_vf_mac, sizeof(ifla_vf_mac)); + if (!rta || !nlAppend(nlm, sizeof(nlmsgbuf), rtattbuf, rta->rta_len)) + goto buffer_too_small; + + rta = rtattrCreate(rtattbuf, sizeof(rtattbuf), IFLA_VF_VLAN, + &ifla_vf_vlan, sizeof(ifla_vf_vlan)); + + if (!rta || !nlAppend(nlm, sizeof(nlmsgbuf), rtattbuf, rta->rta_len)) + goto buffer_too_small; + + vfinfo->rta_len = (char *)nlm + nlm->nlmsg_len - (char *)vfinfo; + + vfinfolist->rta_len = (char *)nlm + nlm->nlmsg_len - + (char *)vfinfolist; + } + + if (vf == PORT_SELF_VF && nltarget_kernel) { + 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 (vfports) { + /* end nesting of vfports */ + vfports->rta_len = (char *)nlm + nlm->nlmsg_len - (char *)vfports; + } + + if (!nltarget_kernel) { + pid = getLldpadPid(); + if (pid == 0) + return -1; + } + + if (nlComm(nlm, &recvbuf, &recvbuflen, pid) < 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, + const char *ifname, int ifindex, + const unsigned char *macaddr, + int vlanid, + 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] = { NULL , }; + int repeats = STATUS_POLL_TIMEOUT_USEC / STATUS_POLL_INTERVL_USEC; + uint16_t status = 0; + bool is8021Qbg = (profileId == NULL); + + rc = doPortProfileOpSetLink(nltarget_kernel, + ifname, ifindex, + macaddr, + vlanid, + 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, NULL, ifindex, tb, &recvbuf); + if (rc) + goto err_exit; + rc = getPortProfileStatus(tb, vf, instanceId, nltarget_kernel, + is8021Qbg, &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 " + "interface %s (%d)"), + status, ifname, 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 */ + + +# ifdef IFLA_VF_PORT_MAX + +static int +getPhysdevAndVlan(const char *ifname, int *root_ifindex, char *root_ifname, + int *vlanid) +{ + int ret; + unsigned int nth; + int ifindex = -1; + + *vlanid = -1; + while (1) { + if ((ret = ifaceGetNthParent(ifindex, ifname, 1, + root_ifindex, root_ifname, &nth))) + return ret; + if (nth == 0) + break; + if (*vlanid == -1) { + if (ifaceGetVlanID(root_ifname, vlanid)) + *vlanid = -1; + } + + ifindex = *root_ifindex; + ifname = NULL; + } + + return 0; +} + +# endif + +static int +doPortProfileOp8021Qbg(const char *ifname, + const unsigned char *macaddr, + const virVirtualPortProfileParamsPtr virtPort, + enum virVirtualPortOp virtPortOp) +{ + int rc; + +# ifndef IFLA_VF_PORT_MAX + + (void)ifname; + (void)macaddr; + (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 vlanid; + int physdev_ifindex = 0; + char physdev_ifname[IFNAMSIZ] = { 0, }; + int vf = PORT_SELF_VF; + + if (getPhysdevAndVlan(ifname, &physdev_ifindex, physdev_ifname, + &vlanid) != 0) { + rc = 1; + goto err_exit; + } + + if (vlanid < 0) + vlanid = 0; + + 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, + physdev_ifname, physdev_ifindex, + macaddr, + vlanid, + NULL, + &portVsi, + virtPort->u.virtPort8021Qbg.instanceID, + NULL, + 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; + int vlanid = -1; + const unsigned char *macaddr = NULL; + + 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, NULL, ifindex, + macaddr, + vlanid, + 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; +} + +/** + * 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 +1550,17 @@ 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 unsigned char *macvtap_macaddr, + 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 +1568,14 @@ associatePortProfileId(const char *macvt break; case VIR_VIRTUALPORT_8021QBG: - + rc = doPortProfileOp8021Qbg(macvtap_ifname, macvtap_macaddr, + virtPort, ASSOCIATE); break; case VIR_VIRTUALPORT_8021QBH: - + rc = doPortProfileOp8021Qbh(linkdev, virtPort, + vmuuid, + ASSOCIATE); break; } @@ -767,19 +1584,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 +1609,18 @@ disassociatePortProfileId(const char *ma break; case VIR_VIRTUALPORT_8021QBG: - + rc = doPortProfileOp8021Qbg(macvtap_ifname, NULL, 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,15 @@ void delMacvtap(const char *ifname, # define MACVTAP_MODE_VEPA_STR "vepa" # define MACVTAP_MODE_BRIDGE_STR "bridge" +int vpAssociatePortProfileId(const char *macvtap_ifname, + const unsigned char *macvtap_macaddr, + 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) Index: libvirt-acl/src/libvirt_private.syms =================================================================== --- libvirt-acl.orig/src/libvirt_private.syms +++ libvirt-acl/src/libvirt_private.syms @@ -695,6 +695,9 @@ virArgvToString; ifaceCtrl; ifaceCheck; ifaceGetIndex; +ifaceGetFlags; +ifaceIsUp; +ifaceGetVlanID; # usb.h usbGetDevice; Index: libvirt-acl/src/util/interface.c =================================================================== --- libvirt-acl.orig/src/util/interface.c +++ libvirt-acl/src/util/interface.c @@ -30,6 +30,8 @@ #ifdef __linux__ # include <linux/if.h> +# include <linux/sockios.h> +# include <linux/if_vlan.h> #endif #include "internal.h" @@ -42,6 +44,85 @@ virReportErrorHelper(NULL, VIR_FROM_NET, code, __FILE__, \ __FUNCTION__, __LINE__, __VA_ARGS__) +#if __linux__ +static int +getFlags(int fd, const char *ifname, struct ifreq *ifr) { + + memset(ifr, 0, sizeof(*ifr)); + + if (virStrncpy(ifr->ifr_name, + ifname, strlen(ifname), sizeof(ifr->ifr_name)) == NULL) + return ENODEV; + + if (ioctl(fd, SIOCGIFFLAGS, ifr) < 0) + return errno; + + return 0; +} + + +/** + * ifaceGetFlags + * + * @ifname : name of the interface + * @flags : pointer to short holding the flags on success + * + * Get the flags of the interface. Returns 0 on success, error code on failure. + */ +int +ifaceGetFlags(const char *ifname, short *flags) { + struct ifreq ifr; + int rc; + int fd = socket(PF_PACKET, SOCK_DGRAM, 0); + + if (fd < 0) + return errno; + + rc = getFlags(fd, ifname, &ifr); + + *flags = ifr.ifr_flags; + + close(fd); + + return rc; +} + + +int +ifaceIsUp(const char *ifname, bool *up) { + short flags; + int rc = ifaceGetFlags(ifname, &flags); + + if (rc) + return rc; + + *up = ((flags & IFF_UP) == IFF_UP); + + return 0; +} +#else + +/* Note: Showstopper on cygwin is only missing PF_PACKET */ + +int +ifaceGetFlags(const char *ifname ATTRIBUTE_UNUSED, + short *flags ATTRIBUTE_UNUSED) { + ifaceError(VIR_ERR_INTERNAL_ERROR, "%s", + _("ifaceGetFlags is not supported on non-linux platforms")); + return ENOSYS; +} + +int +ifaceIsUp(const char *ifname ATTRIBUTE_UNUSED, + bool *up ATTRIBUTE_UNUSED) { + + ifaceError(VIR_ERR_INTERNAL_ERROR, "%s", + _("ifaceIsUp is not supported on non-linux platforms")); + return ENOSYS; +} + +#endif /* __linux__ */ + /* * chgIfaceFlags: Change flags on an interface * @@ -59,23 +140,16 @@ static int chgIfaceFlags(const char *ifname, short flagclear, short flagset) { struct ifreq ifr; int rc = 0; - int flags; + short flags; short flagmask = (~0 ^ flagclear); int fd = socket(PF_PACKET, SOCK_DGRAM, 0); if (fd < 0) return errno; - if (virStrncpy(ifr.ifr_name, - ifname, strlen(ifname), sizeof(ifr.ifr_name)) == NULL) { - rc = ENODEV; + rc = getFlags(fd, ifname, &ifr); + if (rc != 0) goto err_exit; - } - - if (ioctl(fd, SIOCGIFFLAGS, &ifr) < 0) { - rc = errno; - goto err_exit; - } flags = (ifr.ifr_flags & flagmask) | flagset; @@ -151,6 +225,8 @@ ifaceCheck(bool reportError, const char if (fd < 0) return errno; + memset(&ifr, 0, sizeof(ifr)); + if (virStrncpy(ifr.ifr_name, ifname, strlen(ifname), sizeof(ifr.ifr_name)) == NULL) { if (reportError) @@ -227,6 +303,8 @@ ifaceGetIndex(bool reportError, const ch if (fd < 0) return errno; + memset(&ifreq, 0, sizeof(ifreq)); + if (virStrncpy(ifreq.ifr_name, ifname, strlen(ifname), sizeof(ifreq.ifr_name)) == NULL) { if (reportError) @@ -269,3 +347,46 @@ ifaceGetIndex(bool reportError, } #endif /* __linux__ */ + +#ifdef __linux__ +int +ifaceGetVlanID(const char *vlanifname, int *vlanid) { + struct vlan_ioctl_args vlanargs = { + .cmd = GET_VLAN_VID_CMD, + }; + int rc = 0; + int fd = socket(PF_PACKET, SOCK_DGRAM, 0); + + if (fd < 0) + return errno; + + if (virStrcpyStatic(vlanargs.device1, vlanifname) == NULL) { + rc = EINVAL; + goto err_exit; + } + + if (ioctl(fd, SIOCGIFVLAN, &vlanargs) != 0) { + rc = errno; + goto err_exit; + } + + *vlanid = vlanargs.u.VID; + + err_exit: + close(fd); + + return rc; +} + +#else + +int +ifaceGetVlanID(const char *vlanifname ATTRIBUTE_UNUSED, + int *vlanid ATTRIBUTE_UNUSED) { + + ifaceError(VIR_ERR_INTERNAL_ERROR, "%s", + _("ifaceGetVlanID is not supported on non-linux platforms")); + + return ENOSYS; +} +#endif /* __linux__ */ Index: libvirt-acl/src/util/interface.h =================================================================== --- libvirt-acl.orig/src/util/interface.h +++ libvirt-acl/src/util/interface.h @@ -12,6 +12,9 @@ # include "datatypes.h" +int ifaceGetFlags(const char *name, short *flags); +int ifaceIsUp(const char *name, bool *up); + int ifaceCtrl(const char *name, bool up); static inline int ifaceUp(const char *name) { @@ -27,4 +30,6 @@ int ifaceCheck(bool reportError, const c int ifaceGetIndex(bool reportError, const char *ifname, int *ifindex); +int ifaceGetVlanID(const char *vlanifname, int *vlanid); + #endif /* __VIR_INTERFACE_H__ */

2 3

[libvirt] [PATCH] build: make cpp indentation conform
by Jim Meyering 29 May '10

29 May '10

FYI, I've pushed this: >From f9a4df5a5bc4ab9a645928430db84ec3807a996e Mon Sep 17 00:00:00 2001 From: Jim Meyering <meyering(a)redhat.com> Date: Sat, 29 May 2010 09:45:21 +0200 Subject: [PATCH] build: make cpp indentation conform * src/storage/storage_backend.h (VIR_STORAGE_VOL_OPEN_DEFAULT): Adjust s/#define/# define/, and align continued lines. --- src/storage/storage_backend.h | 8 ++++---- 1 files changed, 4 insertions(+), 4 deletions(-) diff --git a/src/storage/storage_backend.h b/src/storage/storage_backend.h index bb4d1c0..1165a45 100644 --- a/src/storage/storage_backend.h +++ b/src/storage/storage_backend.h @@ -93,10 +93,10 @@ enum { VIR_STORAGE_VOL_OPEN_CHAR = 1 << 3, /* char files okay */ }; -#define VIR_STORAGE_VOL_OPEN_DEFAULT (VIR_STORAGE_VOL_OPEN_ERROR |\ - VIR_STORAGE_VOL_OPEN_REG |\ - VIR_STORAGE_VOL_OPEN_CHAR |\ - VIR_STORAGE_VOL_OPEN_BLOCK) +# define VIR_STORAGE_VOL_OPEN_DEFAULT (VIR_STORAGE_VOL_OPEN_ERROR |\ + VIR_STORAGE_VOL_OPEN_REG |\ + VIR_STORAGE_VOL_OPEN_CHAR |\ + VIR_STORAGE_VOL_OPEN_BLOCK) int virStorageBackendVolOpenCheckMode(const char *path, unsigned int flags) ATTRIBUTE_RETURN_CHECK -- 1.7.1.348.gb26ba

1 0

[libvirt] [PATCH v4] storage: Check for invalid storage mode before opening
by Cole Robinson 28 May '10

28 May '10

If a directory pool contains pipes or sockets, a pool start can fail or hang: https://bugzilla.redhat.com/show_bug.cgi?id=589577 We already try to avoid these special files, but only attempt after opening the path, which is where the problems lie. Unify volume opening into helper functions, which use the proper open() flags to avoid error, followed by fstat to validate storage mode. Previously, virStorageBackendUpdateVolTargetInfoFD attempted to enforce the storage mode check, but allowed callers to detect this case and silently continue. In practice, only the FS backend was using this feature, the rest were treating unknown mode as an error condition. Unfortunately the InfoFD function wasn't raising an error message here, so error reporting was busted. This patch adds 2 functions: virStorageBackendVolOpen, and virStorageBackendVolOpenModeSkip. The latter retains the original opt out semantics, the former now throws an explicit error. This patch maintains the previous volume mode checks: allowing specific modes for specific pool types requires a bit of surgery, since VolOpen is called through several different helper functions. v2: Use ATTRIBUTE_NONNULL. Drop stat check, just open with O_NONBLOCK|O_NOCTTY. v3: Move mode check logic back to VolOpen. Use 2 VolOpen functions with different error semantics. v4: Make second VolOpen function more extensible. Didn't opt to change FS backend defaults, this can just be to fix the original bug. Signed-off-by: Cole Robinson <crobinso(a)redhat.com> --- src/storage/storage_backend.c | 72 ++++++++++++++++++++++++++------ src/storage/storage_backend.h | 19 +++++++++ src/storage/storage_backend_fs.c | 13 +++--- src/storage/storage_backend_logical.c | 9 +--- src/storage/storage_backend_mpath.c | 9 +--- src/storage/storage_backend_scsi.c | 17 ++++---- 6 files changed, 97 insertions(+), 42 deletions(-) diff --git a/src/storage/storage_backend.c b/src/storage/storage_backend.c index f4124df..6620592 100644 --- a/src/storage/storage_backend.c +++ b/src/storage/storage_backend.c @@ -873,20 +873,70 @@ virStorageBackendForType(int type) { } +/* + * Allows caller to silently ignore files with improper mode + * + * Returns -1 on error, -2 if file mode is unexpected. + */ int -virStorageBackendUpdateVolTargetInfo(virStorageVolTargetPtr target, - unsigned long long *allocation, - unsigned long long *capacity) +virStorageBackendVolOpenCheckMode(const char *path, unsigned int flags) { - int ret, fd; + int fd, mode = 0; + struct stat sb; - if ((fd = open(target->path, O_RDONLY)) < 0) { + if ((fd = open(path, O_RDONLY|O_NONBLOCK|O_NOCTTY)) < 0) { virReportSystemError(errno, _("cannot open volume '%s'"), - target->path); + path); return -1; } + if (fstat(fd, &sb) < 0) { + virReportSystemError(errno, + _("cannot stat file '%s'"), + path); + close(fd); + return -1; + } + + if (S_ISREG(sb.st_mode)) + mode = VIR_STORAGE_VOL_OPEN_REG; + else if (S_ISCHR(sb.st_mode)) + mode = VIR_STORAGE_VOL_OPEN_CHAR; + else if (S_ISBLK(sb.st_mode)) + mode = VIR_STORAGE_VOL_OPEN_BLOCK; + + if (!(mode & flags)) { + close(fd); + + if (mode & VIR_STORAGE_VOL_OPEN_ERROR) { + virStorageReportError(VIR_ERR_INTERNAL_ERROR, + _("unexpected storage mode for '%s'"), path); + return -1; + } + + return -2; + } + + return fd; +} + +int virStorageBackendVolOpen(const char *path) +{ + return virStorageBackendVolOpenCheckMode(path, DEFAULT_VOL_OPEN_FLAGS); +} + +int +virStorageBackendUpdateVolTargetInfo(virStorageVolTargetPtr target, + unsigned long long *allocation, + unsigned long long *capacity) +{ + int ret, fd; + + if ((ret = virStorageBackendVolOpen(target->path)) < 0) + return ret; + + fd = ret; ret = virStorageBackendUpdateVolTargetInfoFD(target, fd, allocation, @@ -920,12 +970,11 @@ virStorageBackendUpdateVolInfo(virStorageVolDefPtr vol, * virStorageBackendUpdateVolTargetInfoFD: * @conn: connection to report errors on * @target: target definition ptr of volume to update - * @fd: fd of storage volume to update + * @fd: fd of storage volume to update, via virStorageBackendOpenVol* * @allocation: If not NULL, updated allocation information will be stored * @capacity: If not NULL, updated capacity info will be stored * - * Returns 0 for success-1 on a legitimate error condition, - * -2 if passed FD isn't a regular, char, or block file. + * Returns 0 for success, -1 on a legitimate error condition. */ int virStorageBackendUpdateVolTargetInfoFD(virStorageVolTargetPtr target, @@ -945,11 +994,6 @@ virStorageBackendUpdateVolTargetInfoFD(virStorageVolTargetPtr target, return -1; } - if (!S_ISREG(sb.st_mode) && - !S_ISCHR(sb.st_mode) && - !S_ISBLK(sb.st_mode)) - return -2; - if (allocation) { if (S_ISREG(sb.st_mode)) { #ifndef WIN32 diff --git a/src/storage/storage_backend.h b/src/storage/storage_backend.h index 907c4bc..d0f40da 100644 --- a/src/storage/storage_backend.h +++ b/src/storage/storage_backend.h @@ -80,6 +80,25 @@ struct _virStorageBackend { virStorageBackendPtr virStorageBackendForType(int type); +int virStorageBackendVolOpen(const char *path) +ATTRIBUTE_NONNULL(1); + +/* VolOpenCheckMode flags */ +enum { + VIR_STORAGE_VOL_OPEN_ERROR = 1 << 0, /* warn if unexpected type + * encountered */ + VIR_STORAGE_VOL_OPEN_REG = 1 << 1, /* regular files okay */ + VIR_STORAGE_VOL_OPEN_BLOCK = 1 << 2, /* block files okay */ + VIR_STORAGE_VOL_OPEN_CHAR = 1 << 3, /* char okay */ +}; + +#define DEFAULT_VOL_OPEN_FLAGS (VIR_STORAGE_VOL_OPEN_ERROR |\ + VIR_STORAGE_VOL_OPEN_REG |\ + VIR_STORAGE_VOL_OPEN_CHAR |\ + VIR_STORAGE_VOL_OPEN_BLOCK) + +int virStorageBackendVolOpenCheckMode(const char *path, unsigned int flags) +ATTRIBUTE_NONNULL(1); int virStorageBackendUpdateVolInfo(virStorageVolDefPtr vol, int withCapacity); diff --git a/src/storage/storage_backend_fs.c b/src/storage/storage_backend_fs.c index c96c4f3..9ff46f8 100644 --- a/src/storage/storage_backend_fs.c +++ b/src/storage/storage_backend_fs.c @@ -61,18 +61,17 @@ virStorageBackendProbeTarget(virStorageVolTargetPtr target, if (encryption) *encryption = NULL; - if ((fd = open(target->path, O_RDONLY)) < 0) { - virReportSystemError(errno, - _("cannot open volume '%s'"), - target->path); - return -1; - } + if ((ret = virStorageBackendVolOpenCheckMode(target->path, + (DEFAULT_VOL_OPEN_FLAGS & + ~VIR_STORAGE_VOL_OPEN_ERROR))) < 0) + return ret; /* Take care to propagate ret, it is not always -1 */ + fd = ret; if ((ret = virStorageBackendUpdateVolTargetInfoFD(target, fd, allocation, capacity)) < 0) { close(fd); - return ret; /* Take care to propagate ret, it is not always -1 */ + return -1; } memset(&meta, 0, sizeof(meta)); diff --git a/src/storage/storage_backend_logical.c b/src/storage/storage_backend_logical.c index 06238d1..616ca1a 100644 --- a/src/storage/storage_backend_logical.c +++ b/src/storage/storage_backend_logical.c @@ -559,7 +559,7 @@ virStorageBackendLogicalCreateVol(virConnectPtr conn, virStoragePoolObjPtr pool, virStorageVolDefPtr vol) { - int fd = -1; + int fdret, fd = -1; char size[100]; const char *cmdargvnew[] = { LVCREATE, "--name", vol->name, "-L", size, @@ -602,12 +602,9 @@ virStorageBackendLogicalCreateVol(virConnectPtr conn, if (virRun(cmdargv, NULL) < 0) return -1; - if ((fd = open(vol->target.path, O_RDONLY)) < 0) { - virReportSystemError(errno, - _("cannot read path '%s'"), - vol->target.path); + if ((fdret = virStorageBackendVolOpen(vol->target.path)) < 0) goto cleanup; - } + fd = fdret; /* We can only chown/grp if root */ if (getuid() == 0) { diff --git a/src/storage/storage_backend_mpath.c b/src/storage/storage_backend_mpath.c index 3a137eb..3d7dfcc 100644 --- a/src/storage/storage_backend_mpath.c +++ b/src/storage/storage_backend_mpath.c @@ -44,14 +44,11 @@ virStorageBackendMpathUpdateVolTargetInfo(virStorageVolTargetPtr target, unsigned long long *capacity) { int ret = -1; - int fd = -1; + int fdret, fd = -1; - if ((fd = open(target->path, O_RDONLY)) < 0) { - virReportSystemError(errno, - _("cannot open volume '%s'"), - target->path); + if ((fdret = virStorageBackendVolOpen(target->path)) < 0) goto out; - } + fd = fdret; if (virStorageBackendUpdateVolTargetInfoFD(target, fd, diff --git a/src/storage/storage_backend_scsi.c b/src/storage/storage_backend_scsi.c index 40f4fd8..71492cf 100644 --- a/src/storage/storage_backend_scsi.c +++ b/src/storage/storage_backend_scsi.c @@ -135,14 +135,12 @@ virStorageBackendSCSIUpdateVolTargetInfo(virStorageVolTargetPtr target, unsigned long long *allocation, unsigned long long *capacity) { - int fd, ret = -1; + int fdret, fd = -1; + int ret = -1; - if ((fd = open(target->path, O_RDONLY)) < 0) { - virReportSystemError(errno, - _("cannot open volume '%s'"), - target->path); - return -1; - } + if ((fdret = virStorageBackendVolOpen(target->path)) < 0) + goto cleanup; + fd = fdret; if (virStorageBackendUpdateVolTargetInfoFD(target, fd, @@ -155,8 +153,9 @@ virStorageBackendSCSIUpdateVolTargetInfo(virStorageVolTargetPtr target, ret = 0; - cleanup: - close(fd); +cleanup: + if (fd >= 0) + close(fd); return ret; } -- 1.6.6.1

2 2

[libvirt] [PATCH v3] storage: Check for invalid storage mode before opening
by Cole Robinson 28 May '10

28 May '10

If a directory pool contains pipes or sockets, a pool start can fail or hang: https://bugzilla.redhat.com/show_bug.cgi?id=589577 We already try to avoid these special files, but only attempt after opening the path, which is where the problems lie. Unify volume opening into helper functions, which use the proper open() flags to avoid error, followed by fstat to validate storage mode. Previously, virStorageBackendUpdateVolTargetInfoFD attempted to enforce the storage mode check, but allowed callers to detect this case and silently continue. In practice, only the FS backend was using this feature, the rest were treating unknown mode as an error condition. Unfortunately the InfoFD function wasn't raising an error message here, so error reporting was busted. This patch adds 2 functions: virStorageBackendVolOpen, and virStorageBackendVolOpenModeSkip. The latter retains the original opt out semantics, the former now throws an explicit error. This patch maintains the previous volume mode checks: allowing specific modes for specific pool types requires a bit of surgery, since VolOpen is called through several different helper functions. v2: Use ATTRIBUTE_NONNULL. Drop stat check, just open with O_NONBLOCK|O_NOCTTY. v3: Move mode check logic back to VolOpen. Use 2 VolOpen functions with different error semantics to improve error reporting. Signed-off-by: Cole Robinson <crobinso(a)redhat.com> --- src/storage/storage_backend.c | 74 ++++++++++++++++++++++++++------ src/storage/storage_backend.h | 5 ++ src/storage/storage_backend_fs.c | 11 ++--- src/storage/storage_backend_logical.c | 9 +--- src/storage/storage_backend_mpath.c | 9 +--- src/storage/storage_backend_scsi.c | 17 ++++---- 6 files changed, 83 insertions(+), 42 deletions(-) diff --git a/src/storage/storage_backend.c b/src/storage/storage_backend.c index f4124df..702870a 100644 --- a/src/storage/storage_backend.c +++ b/src/storage/storage_backend.c @@ -872,6 +872,61 @@ virStorageBackendForType(int type) { return NULL; } +static int +virStorageBackendVolOpenInternal(const char *path) +{ + int fd; + struct stat sb; + + if ((fd = open(path, O_RDONLY|O_NONBLOCK|O_NOCTTY)) < 0) { + virReportSystemError(errno, + _("cannot open volume '%s'"), + path); + return -1; + } + + if (fstat(fd, &sb) < 0) { + virReportSystemError(errno, + _("cannot stat file '%s'"), + path); + close(fd); + return -1; + } + + if (!S_ISREG(sb.st_mode) && + !S_ISCHR(sb.st_mode) && + !S_ISBLK(sb.st_mode)) { + close(fd); + return -2; + } + + return fd; +} + +/* + * Allows caller to silently ignore files with improper mode + * + * Returns -1 on error, -2 if file mode is unexpected. + */ +int virStorageBackendVolOpenModeSkip(const char *path) +{ + return virStorageBackendVolOpenInternal(path); +} + +int +virStorageBackendVolOpen(const char *path) +{ + int ret; + + ret = virStorageBackendVolOpenInternal(path); + if (ret == -2) { + virStorageReportError(VIR_ERR_INTERNAL_ERROR, + _("unexpected storage mode for '%s'"), path); + return -1; + } + + return ret; +} int virStorageBackendUpdateVolTargetInfo(virStorageVolTargetPtr target, @@ -880,13 +935,10 @@ virStorageBackendUpdateVolTargetInfo(virStorageVolTargetPtr target, { int ret, fd; - if ((fd = open(target->path, O_RDONLY)) < 0) { - virReportSystemError(errno, - _("cannot open volume '%s'"), - target->path); - return -1; - } + if ((ret = virStorageBackendVolOpen(target->path)) < 0) + return ret; + fd = ret; ret = virStorageBackendUpdateVolTargetInfoFD(target, fd, allocation, @@ -920,12 +972,11 @@ virStorageBackendUpdateVolInfo(virStorageVolDefPtr vol, * virStorageBackendUpdateVolTargetInfoFD: * @conn: connection to report errors on * @target: target definition ptr of volume to update - * @fd: fd of storage volume to update + * @fd: fd of storage volume to update, via virStorageBackendOpenVol* * @allocation: If not NULL, updated allocation information will be stored * @capacity: If not NULL, updated capacity info will be stored * - * Returns 0 for success-1 on a legitimate error condition, - * -2 if passed FD isn't a regular, char, or block file. + * Returns 0 for success, -1 on a legitimate error condition. */ int virStorageBackendUpdateVolTargetInfoFD(virStorageVolTargetPtr target, @@ -945,11 +996,6 @@ virStorageBackendUpdateVolTargetInfoFD(virStorageVolTargetPtr target, return -1; } - if (!S_ISREG(sb.st_mode) && - !S_ISCHR(sb.st_mode) && - !S_ISBLK(sb.st_mode)) - return -2; - if (allocation) { if (S_ISREG(sb.st_mode)) { #ifndef WIN32 diff --git a/src/storage/storage_backend.h b/src/storage/storage_backend.h index 907c4bc..8f54dce 100644 --- a/src/storage/storage_backend.h +++ b/src/storage/storage_backend.h @@ -80,6 +80,11 @@ struct _virStorageBackend { virStorageBackendPtr virStorageBackendForType(int type); +int virStorageBackendVolOpen(const char *path) +ATTRIBUTE_NONNULL(1); + +int virStorageBackendVolOpenModeSkip(const char *path) +ATTRIBUTE_NONNULL(1); int virStorageBackendUpdateVolInfo(virStorageVolDefPtr vol, int withCapacity); diff --git a/src/storage/storage_backend_fs.c b/src/storage/storage_backend_fs.c index c96c4f3..5aceb39 100644 --- a/src/storage/storage_backend_fs.c +++ b/src/storage/storage_backend_fs.c @@ -61,18 +61,15 @@ virStorageBackendProbeTarget(virStorageVolTargetPtr target, if (encryption) *encryption = NULL; - if ((fd = open(target->path, O_RDONLY)) < 0) { - virReportSystemError(errno, - _("cannot open volume '%s'"), - target->path); - return -1; - } + if ((ret = virStorageBackendVolOpenModeSkip(target->path)) < 0) + return ret; /* Take care to propagate ret, it is not always -1 */ + fd = ret; if ((ret = virStorageBackendUpdateVolTargetInfoFD(target, fd, allocation, capacity)) < 0) { close(fd); - return ret; /* Take care to propagate ret, it is not always -1 */ + return -1; } memset(&meta, 0, sizeof(meta)); diff --git a/src/storage/storage_backend_logical.c b/src/storage/storage_backend_logical.c index 06238d1..616ca1a 100644 --- a/src/storage/storage_backend_logical.c +++ b/src/storage/storage_backend_logical.c @@ -559,7 +559,7 @@ virStorageBackendLogicalCreateVol(virConnectPtr conn, virStoragePoolObjPtr pool, virStorageVolDefPtr vol) { - int fd = -1; + int fdret, fd = -1; char size[100]; const char *cmdargvnew[] = { LVCREATE, "--name", vol->name, "-L", size, @@ -602,12 +602,9 @@ virStorageBackendLogicalCreateVol(virConnectPtr conn, if (virRun(cmdargv, NULL) < 0) return -1; - if ((fd = open(vol->target.path, O_RDONLY)) < 0) { - virReportSystemError(errno, - _("cannot read path '%s'"), - vol->target.path); + if ((fdret = virStorageBackendVolOpen(vol->target.path)) < 0) goto cleanup; - } + fd = fdret; /* We can only chown/grp if root */ if (getuid() == 0) { diff --git a/src/storage/storage_backend_mpath.c b/src/storage/storage_backend_mpath.c index 3a137eb..3d7dfcc 100644 --- a/src/storage/storage_backend_mpath.c +++ b/src/storage/storage_backend_mpath.c @@ -44,14 +44,11 @@ virStorageBackendMpathUpdateVolTargetInfo(virStorageVolTargetPtr target, unsigned long long *capacity) { int ret = -1; - int fd = -1; + int fdret, fd = -1; - if ((fd = open(target->path, O_RDONLY)) < 0) { - virReportSystemError(errno, - _("cannot open volume '%s'"), - target->path); + if ((fdret = virStorageBackendVolOpen(target->path)) < 0) goto out; - } + fd = fdret; if (virStorageBackendUpdateVolTargetInfoFD(target, fd, diff --git a/src/storage/storage_backend_scsi.c b/src/storage/storage_backend_scsi.c index 40f4fd8..71492cf 100644 --- a/src/storage/storage_backend_scsi.c +++ b/src/storage/storage_backend_scsi.c @@ -135,14 +135,12 @@ virStorageBackendSCSIUpdateVolTargetInfo(virStorageVolTargetPtr target, unsigned long long *allocation, unsigned long long *capacity) { - int fd, ret = -1; + int fdret, fd = -1; + int ret = -1; - if ((fd = open(target->path, O_RDONLY)) < 0) { - virReportSystemError(errno, - _("cannot open volume '%s'"), - target->path); - return -1; - } + if ((fdret = virStorageBackendVolOpen(target->path)) < 0) + goto cleanup; + fd = fdret; if (virStorageBackendUpdateVolTargetInfoFD(target, fd, @@ -155,8 +153,9 @@ virStorageBackendSCSIUpdateVolTargetInfo(virStorageVolTargetPtr target, ret = 0; - cleanup: - close(fd); +cleanup: + if (fd >= 0) + close(fd); return ret; } -- 1.6.6.1

2 3

[libvirt] [Reminder] KVM Forum 2010: Early Bird Registration
by KVM Forum 2010 Program Committee 28 May '10

28 May '10

Just a reminder...The early bird registration period ends on May 30th. It's shaping up to be an excellent KVM Forum, look forward to seeing you there. Registration link is here: http://events.linuxfoundation.org/component/registrationpro/?func=details&d… thanks, -KVM Forum 2010 Program Commitee

1 0

[libvirt] [PATCH] macvtap: cannot support target device name
by Stefan Berger 28 May '10

28 May '10

Since the macvtap device needs active tear-down and the teardown logic is based on the interface name, it can happen that if for example 1 out of 3 interfaces was successfully created, that during the failure path the macvtap's target device name is used to tear down an interface that is doesn't own (owned by another VM). So, in this patch, the target interface name is reset so that there is no target interface name and the interface name is always cleared after a tear down. Signed-off-by: Stefan Berger <stefanb(a)us.ibm.com> Index: libvirt-acl/src/conf/domain_conf.c =================================================================== --- libvirt-acl.orig/src/conf/domain_conf.c +++ libvirt-acl/src/conf/domain_conf.c @@ -2207,6 +2207,8 @@ virDomainNetDefParseXML(virCapsPtr caps, def->data.direct.linkdev = dev; dev = NULL; + VIR_FREE(ifname); + break; case VIR_DOMAIN_NET_TYPE_USER: Index: libvirt-acl/src/qemu/qemu_conf.c =================================================================== --- libvirt-acl.orig/src/qemu/qemu_conf.c +++ libvirt-acl/src/qemu/qemu_conf.c @@ -1556,6 +1556,7 @@ qemudPhysIfaceConnect(virConnectPtr conn rc = -1; delMacvtap(net->ifname, &net->data.direct.virtPortProfile); + VIR_FREE(net->ifname); } } } Index: libvirt-acl/src/qemu/qemu_driver.c =================================================================== --- libvirt-acl.orig/src/qemu/qemu_driver.c +++ libvirt-acl/src/qemu/qemu_driver.c @@ -3708,9 +3708,11 @@ static void qemudShutdownVMDaemon(struct def = vm->def; for (i = 0; i < def->nnets; i++) { virDomainNetDefPtr net = def->nets[i]; - if (net->type == VIR_DOMAIN_NET_TYPE_DIRECT) + if (net->type == VIR_DOMAIN_NET_TYPE_DIRECT) { delMacvtap(net->ifname, &net->data.direct.virtPortProfile); + VIR_FREE(net->ifname); + } } #endif @@ -8545,9 +8547,11 @@ qemudDomainDetachNetDevice(struct qemud_ virNWFilterTearNWFilter(detach); #if WITH_MACVTAP - if (detach->type == VIR_DOMAIN_NET_TYPE_DIRECT) + if (detach->type == VIR_DOMAIN_NET_TYPE_DIRECT) { delMacvtap(detach->ifname, &detach->data.direct.virtPortProfile); + VIR_FREE(detach->ifname); + } #endif if ((driver->macFilter) && (detach->ifname != NULL)) {

2 2

[libvirt] [PATCH] qemu: Add a qemu.conf option for clearing capabilities
by Cole Robinson 28 May '10

28 May '10

Currently there is no way to opt out of libvirt dropping POSIX capabilities for qemu. This at least is a useful debugging tool, but is also wanted by users (and distributors): https://bugzilla.redhat.com/show_bug.cgi?id=559154 https://bugzilla.redhat.com/show_bug.cgi?id=573850 Signed-off-by: Cole Robinson <crobinso(a)redhat.com> --- src/qemu/qemu.conf | 5 +++++ src/qemu/qemu_conf.c | 5 +++++ src/qemu/qemu_conf.h | 2 +- src/qemu/qemu_driver.c | 11 +++++++++-- 4 files changed, 20 insertions(+), 3 deletions(-) diff --git a/src/qemu/qemu.conf b/src/qemu/qemu.conf index 98a1176..b976dcc 100644 --- a/src/qemu/qemu.conf +++ b/src/qemu/qemu.conf @@ -178,3 +178,8 @@ # QEMU_AUDIO_DRV environment variable when using VNC. # # vnc_allow_host_audio = 0 +# + +# If clear_emulator_capabilities is enabled, libvirt will drop all POSIX +# capabilities of the QEmu/KVM emulator. This is enabled by default +# clear_emulator_capabilities = 1 diff --git a/src/qemu/qemu_conf.c b/src/qemu/qemu_conf.c index 66a949e..d7e61d2 100644 --- a/src/qemu/qemu_conf.c +++ b/src/qemu/qemu_conf.c @@ -104,6 +104,7 @@ int qemudLoadDriverConfig(struct qemud_driver *driver, /* Setup critical defaults */ driver->dynamicOwnership = 1; + driver->clearEmulatorCapabilities = 1; if (!(driver->vncListen = strdup("127.0.0.1"))) { virReportOOMError(); @@ -355,6 +356,10 @@ int qemudLoadDriverConfig(struct qemud_driver *driver, CHECK_TYPE ("vnc_allow_host_audio", VIR_CONF_LONG); if (p) driver->vncAllowHostAudio = p->l; + p = virConfGetValue (conf, "clear_emulator_capabilities"); + CHECK_TYPE ("clear_emulator_capabilities", VIR_CONF_LONG); + if (p) driver->clearEmulatorCapabilities = p->l; + virConfFree (conf); return 0; } diff --git a/src/qemu/qemu_conf.h b/src/qemu/qemu_conf.h index 7616d15..0f8a1b3 100644 --- a/src/qemu/qemu_conf.h +++ b/src/qemu/qemu_conf.h @@ -138,8 +138,8 @@ struct qemud_driver { ebtablesContext *ebtables; unsigned int relaxedACS : 1; - unsigned int vncAllowHostAudio : 1; + unsigned int clearEmulatorCapabilities : 1; virCapsPtr caps; diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index ad7fb54..3faf31e 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -3287,7 +3287,7 @@ static int qemudStartVMDaemon(virConnectPtr conn, int stdin_fd) { const char **argv = NULL, **tmp; const char **progenv = NULL; - int i, ret; + int i, ret, runflags; struct stat sb; int *vmfds = NULL; int nvmfds = 0; @@ -3501,9 +3501,16 @@ static int qemudStartVMDaemon(virConnectPtr conn, for (i = 0 ; i < nvmfds ; i++) FD_SET(vmfds[i], &keepfd); + VIR_DEBUG("Clear emulator capabilities: %d", + driver->clearEmulatorCapabilities); + runflags = VIR_EXEC_NONBLOCK; + if (driver->clearEmulatorCapabilities) { + runflags |= VIR_EXEC_CLEAR_CAPS; + } + ret = virExecDaemonize(argv, progenv, &keepfd, &child, stdin_fd, &logfile, &logfile, - VIR_EXEC_NONBLOCK | VIR_EXEC_CLEAR_CAPS, + runflags, qemudSecurityHook, &hookData, pidfile); VIR_FREE(pidfile); -- 1.6.6.1

2 3

[libvirt] [PATCH 0/1] Improve the nodedev tree relationships
by David Allan 28 May '10

28 May '10

The parent/child relationships in the udev node device backend are really kind of gross. If the parent of a device doesn't exist in the libvirt node device tree, the current code sets the parent to the root "computer" node. The result is that we have a lot of devices hanging directly off the root node and relationships like scsi target to scsi hba are broken. The attached patch simply changes the behavior to keep walking up the udev device tree until it finds an ancestor that exists in the libvirt node device tree, or until it actually reaches the root of the tree. The resulting tree is a lot cleaner and more informative, especially on systems with lots of devices. David Allan (1): Improve nodedev parent/child relationships src/node_device/node_device_udev.c | 54 ++++++++++++++++++++++-------------- 1 files changed, 33 insertions(+), 21 deletions(-)

4 4

[libvirt] [PATCH] Fix libvirt-guests init script
by Jiri Denemark 28 May '10

28 May '10

Firstly, the init script has to touch its file under /var/lock/subsys when started, otherwise system would think it's not running and won't stop it during shutdown. Secondly, for some reason there is a policy to automatically enable init scripts when they are installed, so let the specfile do this. We also need to start the init script to ensure it will be stopped during the first shutdown after installing the package. --- daemon/libvirt-guests.init.in | 11 ++++++++++- libvirt.spec.in | 17 ++++++++++++++++- 2 files changed, 26 insertions(+), 2 deletions(-) diff --git a/daemon/libvirt-guests.init.in b/daemon/libvirt-guests.init.in index b87c11b..a08ed82 100644 --- a/daemon/libvirt-guests.init.in +++ b/daemon/libvirt-guests.init.in @@ -37,6 +37,7 @@ SHUTDOWN_TIMEOUT=0 test -f "$sysconfdir"/sysconfig/libvirt-guests && . "$sysconfdir"/sysconfig/libvirt-guests LISTFILE="$localstatedir"/lib/libvirt/libvirt-guests +VAR_SUBSYS_LIBVIRT_GUESTS="$localstatedir"/lock/subsys/libvirt-guests RETVAL=0 @@ -117,12 +118,17 @@ guest_is_on() { return 0 } +started() { + touch $VAR_SUBSYS_LIBVIRT_GUESTS +} + start() { - [ -f $LISTFILE ] || return 0 + [ -f $LISTFILE ] || { started; return 0; } if [ "x$ON_BOOT" != xstart ]; then echo $"libvirt-guests is configured not to start any guests on boot" rm -f $LISTFILE + started return 0 fi @@ -155,6 +161,7 @@ start() { done <$LISTFILE rm -f $LISTFILE + started } suspend_guest() @@ -257,6 +264,8 @@ stop() { fi done done <$LISTFILE + + rm -f $VAR_SUBSYS_LIBVIRT_GUESTS } gueststatus() { diff --git a/libvirt.spec.in b/libvirt.spec.in index b8bd9ab..6edbf2f 100644 --- a/libvirt.spec.in +++ b/libvirt.spec.in @@ -707,7 +707,22 @@ if [ $1 = 0 ]; then fi %endif -%post client -p /sbin/ldconfig +%preun client + +if [ $1 = 0 ]; then + /sbin/chkconfig --del libvirt-guests + rm -f /var/lib/libvirt/libvirt-guests +fi + +%post client + +/sbin/ldconfig +/sbin/chkconfig --add libvirt-guests +if [ $1 -ge 1 ]; then + # this doesn't do anything but allowing for libvirt-guests to be + # stopped on the first shutdown + /sbin/service libvirt-guests start > /dev/null 2>&1 || true +fi %postun client -p /sbin/ldconfig -- 1.7.1

2 3

[libvirt] [PATCH] Prefer UDEV to HAL drivers if both a compiled
by Daniel P. Berrange 28 May '10

28 May '10

HAL is deprecated and UDEV is the future. Thus if both options are compiled, we should prefer use of UDEV over HAL * src/node_device/node_device_driver.c: Switch init order to try UDEV first, then HAL --- src/node_device/node_device_driver.c | 4 ++-- 1 files changed, 2 insertions(+), 2 deletions(-) diff --git a/src/node_device/node_device_driver.c b/src/node_device/node_device_driver.c index a6c1fa0..42363be 100644 --- a/src/node_device/node_device_driver.c +++ b/src/node_device/node_device_driver.c @@ -658,8 +658,8 @@ void registerCommonNodeFuncs(virDeviceMonitorPtr driver) int nodedevRegister(void) { #if defined(HAVE_HAL) && defined(HAVE_UDEV) /* Register only one of these two - they conflict */ - if (halNodeRegister() == -1) - return udevNodeRegister(); + if (udevNodeRegister() == -1) + return halNodeRegister(); return 0; #else # ifdef HAVE_HAL -- 1.6.6.1

3 2