- Devel - libvirt List Archives

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

29 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 29 May '10

29 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 29 May '10

29 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 29 May '10

29 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 29 May '10

29 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 29 May '10

29 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

Re: [libvirt] [Fwd: [PATCH v9] add 802.1Qbh and 802.1Qbg handling]
by Arnd Bergmann 28 May '10

28 May '10

On Thursday 27 May 2010, Stefan Berger wrote: > +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; > + } > + } > + } > + } There may be multiple IFLA_VF_PORT attributes in the IFLA_VF_PORTS list, so you cannot do nla_parse_nested. I think this should be nla_for_each_attr instead, and compare the uuid to the one you expect. > + 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; > + } This part looks good now. > + 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); > + } But this still goes down the IFLA_PORT_SELF route because you pass PORT_SELF_VF even for nltarget_kernel==false, where it makes no sense. Maybe make the above if (vf == PORT_SELF_VF && nltarget_kernel) > + if (vf != PORT_SELF_VF) { > + /* end nesting of vfports */ > + vfports->rta_len = (char *)nlm + nlm->nlmsg_len - (char *)vfports; > + } Here too. > + 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; > + } I don't understand this part yet. Do we need this difference? > + while (--repeats >= 0) { > + rc = link_dump(nltarget_kernel, NULL, 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; > + } Hmm, we seem to be missing an INPROGRESS status for Qbg. Any suggestions what we should return there? Should we possibly just leave out IFLA_PORT_RESPONSE in order to signal INPROGRESS, as in not clear yet? > + rc = doPortProfileOpCommon(nltarget_kernel, > + physdev_ifname, physdev_ifindex, > + macaddr, > + vlanid, > + NULL, > + &portVsi, > + virtPort->u.virtPort8021Qbg.instanceID, > + NULL, > + PORT_SELF_VF, > + op); This is where we pass PORT_SELF_VF together with nltarget_kernel=false, as mentioned above. Arnd

3 9

[libvirt] [PATCH 0/3] Misc fixes to QEMU driver PCI hotplug handling
by Daniel P. Berrange 28 May '10

28 May '10

In testing PCI hotplug I found a few problems - The security driver would overwrite the real error message with a useless one of its own - We didn't correctly detect errors in the non-JSON monitor - We never re-attached the PCI dev to the host if hotplug failed to work.

2 6

[libvirt] [PATCH 0/3] Misc fixes to storage pool & network drivers
by Daniel P. Berrange 28 May '10

28 May '10

These three patches fix misc problems in the storage pool and network drivers identified by the TCK Daniel

3 8