July 2013 - Devel - libvirt List Archives

[libvirt] [PATCH 0/4] Add ability to tune "host-model" cpu flags
by Peter Krempa 16 Jul '13

16 Jul '13

This series adds the ability to influence cpu flags that are created when using "host-model" cpu mode and a cleans up a few places that I went through while fixing the issue. Peter Krempa (4): conf: Clean up error reporting in cpu definition parsing cpu: Add virCPUDefUpdateFeature() cpu: Clean up code style cpu: Allow fine tuning of "host-model" cpu docs/formatdomain.html.in | 14 ++-- src/conf/cpu_conf.c | 160 ++++++++++++++++++++++++++-------------------- src/conf/cpu_conf.h | 5 ++ src/cpu/cpu_x86.c | 49 ++++++++++++-- src/libvirt_private.syms | 1 + 5 files changed, 148 insertions(+), 81 deletions(-) -- 1.8.3.2

2 9

[libvirt] Regarding libvirtd crash in virDomainLookupByName(..) similar to bug 903238/915353/924756
by Udayan Bapat (ubapat) 16 Jul '13

16 Jul '13

Hello All, I work in Cisco Virtualization team and I found this email list from libvirt.org We are hitting a random libvirtd crash with a traceback during guest shutdown - #3 0x00007f4a34a2d4ad in malloc_printerr (action=2, str=0x7f4a34acb682 "corrupted double-linked list", ptr=<value optimized out>) at malloc.c:5949 #4 0x00007f4a34a2d58c in malloc_consolidate (av=<value optimized out>) at malloc.c:4852 #5 0x00007f4a34a2f83c in _int_malloc (av=0x7f4a34cffa20, bytes=262320) at malloc.c:4184 #6 0x00007f4a34a310dc in __libc_calloc (n=<value optimized out>, elem_size=<value optimized out>) at malloc.c:3901 #7 0x00007f4a3620469f in virAlloc () from /auto/mcp-project97/mcp_dev/BLD-BLD_MCP_DEV_LATEST_20130609_000113/linkfarm/x86_64/usr/lib64/libvirt.so.0 #8 0x00007f4a3626e93c in prepareCall () from /auto/mcp-project97/mcp_dev/BLD-BLD_MCP_DEV_LATEST_20130609_000113/linkfarm/x86_64/usr/lib64/libvirt.so.0 #9 0x00007f4a36271463 in call () from /auto/mcp-project97/mcp_dev/BLD-BLD_MCP_DEV_LATEST_20130609_000113/linkfarm/x86_64/usr/lib64/libvirt.so.0 #10 0x00007f4a362624d2 in remoteDomainLookupByName () from /auto/mcp-project97/mcp_dev/BLD-BLD_MCP_DEV_LATEST_20130609_000113/linkfarm/x86_64/usr/lib64/libvirt.so.0 #11 0x00007f4a3624d09f in virDomainLookupByName () from /auto/mcp-project97/mcp_dev/BLD-BLD_MCP_DEV_LATEST_20130609_000113/linkfarm/x86_64/usr/lib64/libvirt.so.0 I believe this TB is quite similar to the one reported by various Redhat bugs mentioned in the subject. We are running libvirt version 0.8.4. I understand this issue has been fixed in the later versions of libvirt but due to business reasons, we are not able to upgrade it yet. Hence I was trying to look for a patch that might have fixed the issue. I found this link that supposedly describes the patch but it is not accessible to people outside red hat network (I think!) https://bugzilla.redhat.com/show_bug.cgi?id=915353 It would be really helpful if you could point me out a patch if it exists! Thank you in advance. Udayan Bapat Cisco – Foundation Engineering Virtualization Research Triangle Park, NC

2 1

[libvirt] [PATCH] lxc: hoist supplemental group detection before clone
by Eric Blake 16 Jul '13

16 Jul '13

Commit 75c1256 states that virGetGroupList must not be called between fork and exec, then commit ee777e99 promptly violated that for lxc. Hoist the group detection to occur before clone. * src/lxc/lxc_container.c (__lxc_child_argv): Add members. (lxcContainerSetID): Adjust signature. (lxcContainerChild, lxcContainerStart): Adjust callers. Signed-off-by: Eric Blake <eblake(a)redhat.com> --- src/lxc/lxc_container.c | 26 +++++++++++++++----------- 1 file changed, 15 insertions(+), 11 deletions(-) diff --git a/src/lxc/lxc_container.c b/src/lxc/lxc_container.c index 940233b..50aaa36 100644 --- a/src/lxc/lxc_container.c +++ b/src/lxc/lxc_container.c @@ -106,6 +106,8 @@ struct __lxc_child_argv { char **ttyPaths; size_t nttyPaths; int handshakefd; + gid_t *groups; + int ngroups; }; static int lxcContainerMountFSBlock(virDomainFSDefPtr fs, @@ -349,26 +351,20 @@ int lxcContainerWaitForContinue(int control) * * Returns 0 on success or -1 in case of error */ -static int lxcContainerSetID(virDomainDefPtr def) +static int +lxcContainerSetID(virDomainDefPtr def, gid_t* groups, int ngroups) { - gid_t *groups; - int ngroups; - /* Only call virSetUIDGID when user namespace is enabled * for this container. And user namespace is only enabled * when nuidmap&ngidmap is not zero */ VIR_DEBUG("Set UID/GID to 0/0"); - if (def->idmap.nuidmap && - ((ngroups = virGetGroupList(0, 0, &groups) < 0) || - virSetUIDGID(0, 0, groups, ngroups) < 0)) { + if (def->idmap.nuidmap && virSetUIDGID(0, 0, groups, ngroups) < 0) { virReportSystemError(errno, "%s", _("setuid or setgid failed")); - VIR_FREE(groups); return -1; } - VIR_FREE(groups); return 0; } @@ -1981,7 +1977,7 @@ static int lxcContainerChild(void *data) cmd = lxcContainerBuildInitCmd(vmDef); virCommandWriteArgLog(cmd, 1); - if (lxcContainerSetID(vmDef) < 0) + if (lxcContainerSetID(vmDef, argv->groups, argv->ngroups) < 0) goto cleanup; root = virDomainGetRootFilesystem(vmDef); @@ -2054,6 +2050,7 @@ cleanup: VIR_FORCE_CLOSE(ttyfd); VIR_FORCE_CLOSE(argv->monitor); VIR_FORCE_CLOSE(argv->handshakefd); + VIR_FREE(argv->groups); if (ret == 0) { /* this function will only return if an error occurred */ @@ -2140,13 +2137,18 @@ int lxcContainerStart(virDomainDefPtr def, char *stack, *stacktop; lxc_child_argv_t args = { def, securityDriver, nveths, veths, control, - ttyPaths, nttyPaths, handshakefd}; + ttyPaths, nttyPaths, handshakefd, NULL, 0 }; /* allocate a stack for the container */ if (VIR_ALLOC_N(stack, stacksize) < 0) return -1; stacktop = stack + stacksize; + if ((args.ngroups = virGetGroupList(0, 0, &args.groups)) < 0) { + VIR_FREE(stack); + return -1; + } + cflags = CLONE_NEWPID|CLONE_NEWNS|CLONE_NEWUTS|CLONE_NEWIPC|SIGCHLD; if (userns_required(def)) { @@ -2157,6 +2159,7 @@ int lxcContainerStart(virDomainDefPtr def, virReportSystemError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", _("Kernel doesn't support user namespace")); VIR_FREE(stack); + VIR_FREE(args.groups); return -1; } } @@ -2168,6 +2171,7 @@ int lxcContainerStart(virDomainDefPtr def, pid = clone(lxcContainerChild, stacktop, cflags, &args); VIR_FREE(stack); + VIR_FREE(args.groups); VIR_DEBUG("clone() completed, new container PID is %d", pid); if (pid < 0) { -- 1.8.1.4

3 4

[libvirt] [PATCH 1/7] LXC: Setup disks for container on host side
by Gao feng 16 Jul '13

16 Jul '13

Since mknod in container is forbidden, we should setup disks on host side. Signed-off-by: Gao feng <gaofeng(a)cn.fujitsu.com> --- src/lxc/lxc_container.c | 98 ------------------------------------------------ src/lxc/lxc_controller.c | 94 ++++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 94 insertions(+), 98 deletions(-) diff --git a/src/lxc/lxc_container.c b/src/lxc/lxc_container.c index fcd9b74..caad02b 100644 --- a/src/lxc/lxc_container.c +++ b/src/lxc/lxc_container.c @@ -1367,100 +1367,6 @@ static int lxcContainerMountAllFS(virDomainDefPtr vmDef, } -static int lxcContainerSetupDisk(virDomainDefPtr vmDef, - virDomainDiskDefPtr def, - virSecurityManagerPtr securityDriver) -{ - char *src = NULL; - char *dst = NULL; - int ret = -1; - struct stat sb; - mode_t mode; - char *tmpsrc = def->src; - - if (def->type != VIR_DOMAIN_DISK_TYPE_BLOCK) { - virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", - _("Can't setup disk for non-block device")); - goto cleanup; - } - if (def->src == NULL) { - virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", - _("Can't setup disk without media")); - goto cleanup; - } - - if (virAsprintf(&src, "/.oldroot/%s", def->src) < 0) - goto cleanup; - - if (virAsprintf(&dst, "/dev/%s", def->dst) < 0) - goto cleanup; - - if (stat(src, &sb) < 0) { - virReportSystemError(errno, - _("Unable to access %s"), def->src); - goto cleanup; - } - - if (!S_ISCHR(sb.st_mode) && !S_ISBLK(sb.st_mode)) { - virReportError(VIR_ERR_CONFIG_UNSUPPORTED, - _("Disk source %s must be a character/block device"), - def->src); - goto cleanup; - } - - mode = 0700; - if (S_ISCHR(sb.st_mode)) - mode |= S_IFCHR; - else - mode |= S_IFBLK; - - /* Yes, the device name we're creating may not - * actually correspond to the major:minor number - * we're using, but we've no other option at this - * time. Just have to hope that containerized apps - * don't get upset that the major:minor is different - * to that normally implied by the device name - */ - VIR_DEBUG("Creating dev %s (%d,%d) from %s", - dst, major(sb.st_rdev), minor(sb.st_rdev), src); - if (mknod(dst, mode, sb.st_rdev) < 0) { - virReportSystemError(errno, - _("Unable to create device %s"), - dst); - goto cleanup; - } - /* Labelling normally operates on src, but we need - * to actally label the dst here, so hack the config */ - def->src = dst; - if (virSecurityManagerSetImageLabel(securityDriver, vmDef, def) < 0) - goto cleanup; - - ret = 0; - -cleanup: - def->src = tmpsrc; - VIR_FREE(src); - VIR_FREE(dst); - return ret; -} - -static int lxcContainerSetupAllDisks(virDomainDefPtr vmDef, - virSecurityManagerPtr securityDriver) -{ - size_t i; - VIR_DEBUG("Setting up disks"); - - for (i = 0; i < vmDef->ndisks; i++) { - if (lxcContainerSetupDisk(vmDef, vmDef->disks[i], - securityDriver) < 0) - return -1; - } - - VIR_DEBUG("Setup all disks"); - return 0; -} - - static int lxcContainerSetupHostdevSubsysUSB(virDomainDefPtr vmDef, virDomainHostdevDefPtr def, virSecurityManagerPtr securityDriver) @@ -1837,10 +1743,6 @@ static int lxcContainerSetupPivotRoot(virDomainDefPtr vmDef, if (lxcContainerMountAllFS(vmDef, sec_mount_options) < 0) goto cleanup; - /* Sets up any extra disks from guest config */ - if (lxcContainerSetupAllDisks(vmDef, securityDriver) < 0) - goto cleanup; - /* Sets up any extra host devices from guest config */ if (lxcContainerSetupAllHostdevs(vmDef, securityDriver) < 0) goto cleanup; diff --git a/src/lxc/lxc_controller.c b/src/lxc/lxc_controller.c index 3f3d93b..e9d2848 100644 --- a/src/lxc/lxc_controller.c +++ b/src/lxc/lxc_controller.c @@ -1309,6 +1309,97 @@ cleanup: } +static int virLXCControllerSetupDisk(virLXCControllerPtr ctrl, + virDomainDiskDefPtr def, + virSecurityManagerPtr securityDriver) +{ + char *dst = NULL; + int ret = -1; + struct stat sb; + mode_t mode; + char *tmpsrc = def->src; + + if (def->type != VIR_DOMAIN_DISK_TYPE_BLOCK) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("Can't setup disk for non-block device")); + goto cleanup; + } + if (def->src == NULL) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("Can't setup disk without media")); + goto cleanup; + } + + if (virAsprintf(&dst, "/%s/%s.dev/%s", + LXC_STATE_DIR, ctrl->def->name, def->dst) < 0) + goto cleanup; + + if (stat(def->src, &sb) < 0) { + virReportSystemError(errno, + _("Unable to access %s"), def->src); + goto cleanup; + } + + if (!S_ISCHR(sb.st_mode) && !S_ISBLK(sb.st_mode)) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, + _("Disk source %s must be a character/block device"), + def->src); + goto cleanup; + } + + mode = 0700; + if (S_ISCHR(sb.st_mode)) + mode |= S_IFCHR; + else + mode |= S_IFBLK; + + /* Yes, the device name we're creating may not + * actually correspond to the major:minor number + * we're using, but we've no other option at this + * time. Just have to hope that containerized apps + * don't get upset that the major:minor is different + * to that normally implied by the device name + */ + VIR_DEBUG("Creating dev %s (%d,%d) from %s", + dst, major(sb.st_rdev), minor(sb.st_rdev), def->src); + if (mknod(dst, mode, sb.st_rdev) < 0) { + virReportSystemError(errno, + _("Unable to create device %s"), + dst); + goto cleanup; + } + + /* Labelling normally operates on src, but we need + * to actally label the dst here, so hack the config */ + def->src = dst; + if (virSecurityManagerSetImageLabel(securityDriver, ctrl->def, def) < 0) + goto cleanup; + + ret = 0; + +cleanup: + def->src = tmpsrc; + VIR_FREE(dst); + return ret; +} + +static int virLXCControllerSetupAllDisks(virLXCControllerPtr ctrl) +{ + size_t i; + VIR_DEBUG("Setting up disks"); + + for (i = 0; i < ctrl->def->ndisks; i++) { + if (virLXCControllerSetupDisk(ctrl, ctrl->def->disks[i], + ctrl->securityManager) < 0) + return -1; + } + + VIR_DEBUG("Setup all disks"); + return 0; +} + + + /** * virLXCControllerMoveInterfaces * @nveths: number of interfaces @@ -1724,6 +1815,9 @@ virLXCControllerRun(virLXCControllerPtr ctrl) if (virLXCControllerPopulateDevices(ctrl) < 0) goto cleanup; + if (virLXCControllerSetupAllDisks(ctrl) < 0) + goto cleanup; + if (virLXCControllerSetupFuse(ctrl) < 0) goto cleanup; -- 1.8.3.1

2 8

[libvirt] [PATCH] security_dac: compute supplemental groups before fork
by Eric Blake 15 Jul '13

15 Jul '13

Commit 75c1256 states that virGetGroupList must not be called between fork and exec, then commit ee777e99 promptly violated that for lxc's use of virSecurityManagerSetProcessLabel. Hoist the supplemental group detection to the time that the security manager is created. Qemu is safe, as it uses virSecurityManagerSetChildProcessLabel which in turn uses virCommand to determine supplemental groups. This does not fix the fact that virSecurityManagerSetProcessLabel calls virSecurityDACParseIds calls parseIds which eventually calls getpwnam_r, which also violates fork/exec async-signal-safe safety rules, but so far no one has complained of hitting deadlock in that case. * src/security/security_dac.c (virSecurityDACSetUser) (virSecurityDACSetGroup): Merge... (virSecurityDACSetUIDGID): ...into new function. (_virSecurityDACData): Track groups in private data. (virSecurityDACClose): Clean up new fields. (virSecurityDACGetIds): Alter signature. (virSecurityDACSetSecurityHostdevLabelHelper) (virSecurityDACSetChardevLabel, virSecurityDACSetProcessLabel) (virSecurityDACSetChildProcessLabel): Update callers. * src/security/security_dac.h: Update prototypes to match. * src/security/security_manager.c (virSecurityManagerNewDAC): Adjust caller. Signed-off-by: Eric Blake <eblake(a)redhat.com> --- src/security/security_dac.c | 59 ++++++++++++++++++++++------------------- src/security/security_dac.h | 9 +++---- src/security/security_manager.c | 11 ++++++-- 3 files changed, 44 insertions(+), 35 deletions(-) diff --git a/src/security/security_dac.c b/src/security/security_dac.c index 9b5eaa8..124f270 100644 --- a/src/security/security_dac.c +++ b/src/security/security_dac.c @@ -43,23 +43,20 @@ typedef virSecurityDACData *virSecurityDACDataPtr; struct _virSecurityDACData { uid_t user; gid_t group; + gid_t *groups; + int ngroups; bool dynamicOwnership; }; void -virSecurityDACSetUser(virSecurityManagerPtr mgr, - uid_t user) +virSecurityDACSetUIDGID(virSecurityManagerPtr mgr, + uid_t user, gid_t group, gid_t *groups, int ngroups) { virSecurityDACDataPtr priv = virSecurityManagerGetPrivateData(mgr); priv->user = user; -} - -void -virSecurityDACSetGroup(virSecurityManagerPtr mgr, - gid_t group) -{ - virSecurityDACDataPtr priv = virSecurityManagerGetPrivateData(mgr); priv->group = group; + priv->groups = groups; + priv->ngroups = ngroups; } void @@ -146,7 +143,8 @@ virSecurityDACParseIds(virDomainDefPtr def, uid_t *uidPtr, gid_t *gidPtr) static int virSecurityDACGetIds(virDomainDefPtr def, virSecurityDACDataPtr priv, - uid_t *uidPtr, gid_t *gidPtr) + uid_t *uidPtr, gid_t *gidPtr, + gid_t **groups, int *ngroups) { int ret; @@ -157,8 +155,13 @@ virSecurityDACGetIds(virDomainDefPtr def, virSecurityDACDataPtr priv, return -1; } - if ((ret = virSecurityDACParseIds(def, uidPtr, gidPtr)) <= 0) + if ((ret = virSecurityDACParseIds(def, uidPtr, gidPtr)) <= 0) { + if (groups) + *groups = NULL; + if (ngroups) + ngroups = 0; return ret; + } if (!priv) { virReportError(VIR_ERR_INTERNAL_ERROR, @@ -171,6 +174,10 @@ virSecurityDACGetIds(virDomainDefPtr def, virSecurityDACDataPtr priv, *uidPtr = priv->user; if (gidPtr) *gidPtr = priv->group; + if (groups) + *groups = priv->groups; + if (ngroups) + *ngroups = priv->ngroups; return 0; } @@ -250,8 +257,10 @@ virSecurityDACOpen(virSecurityManagerPtr mgr ATTRIBUTE_UNUSED) } static int -virSecurityDACClose(virSecurityManagerPtr mgr ATTRIBUTE_UNUSED) +virSecurityDACClose(virSecurityManagerPtr mgr) { + virSecurityDACDataPtr priv = virSecurityManagerGetPrivateData(mgr); + VIR_FREE(priv->groups); return 0; } @@ -448,7 +457,7 @@ virSecurityDACSetSecurityHostdevLabelHelper(const char *file, uid_t user; gid_t group; - if (virSecurityDACGetIds(def, priv, &user, &group)) + if (virSecurityDACGetIds(def, priv, &user, &group, NULL, NULL)) return -1; return virSecurityDACSetOwnership(file, user, group); @@ -702,7 +711,7 @@ virSecurityDACSetChardevLabel(virSecurityManagerPtr mgr, uid_t user; gid_t group; - if (virSecurityDACGetIds(def, priv, &user, &group)) + if (virSecurityDACGetIds(def, priv, &user, &group, NULL, NULL)) return -1; switch (dev->type) { @@ -996,26 +1005,20 @@ virSecurityDACSetProcessLabel(virSecurityManagerPtr mgr, { uid_t user; gid_t group; - virSecurityDACDataPtr priv = virSecurityManagerGetPrivateData(mgr); gid_t *groups; int ngroups; - int ret = -1; + virSecurityDACDataPtr priv = virSecurityManagerGetPrivateData(mgr); - if (virSecurityDACGetIds(def, priv, &user, &group)) - return -1; - ngroups = virGetGroupList(user, group, &groups); - if (ngroups < 0) + if (virSecurityDACGetIds(def, priv, &user, &group, &groups, &ngroups)) return -1; - VIR_DEBUG("Dropping privileges of DEF to %u:%u", - (unsigned int) user, (unsigned int) group); + VIR_DEBUG("Dropping privileges of DEF to %u:%u, %d supplemental groups", + (unsigned int) user, (unsigned int) group, ngroups); if (virSetUIDGID(user, group, groups, ngroups) < 0) - goto cleanup; - ret = 0; -cleanup: - VIR_FREE(groups); - return ret; + return -1; + + return 0; } @@ -1028,7 +1031,7 @@ virSecurityDACSetChildProcessLabel(virSecurityManagerPtr mgr, gid_t group; virSecurityDACDataPtr priv = virSecurityManagerGetPrivateData(mgr); - if (virSecurityDACGetIds(def, priv, &user, &group)) + if (virSecurityDACGetIds(def, priv, &user, &group, NULL, NULL)) return -1; VIR_DEBUG("Setting child to drop privileges of DEF to %u:%u", diff --git a/src/security/security_dac.h b/src/security/security_dac.h index 02432a5..23fcf2c 100644 --- a/src/security/security_dac.h +++ b/src/security/security_dac.h @@ -1,5 +1,5 @@ /* - * Copyright (C) 2010-2011 Red Hat, Inc. + * Copyright (C) 2010-2011, 2013 Red Hat, Inc. * * This library is free software; you can redistribute it and/or * modify it under the terms of the GNU Lesser General Public @@ -25,10 +25,9 @@ extern virSecurityDriver virSecurityDriverDAC; -void virSecurityDACSetUser(virSecurityManagerPtr mgr, - uid_t user); -void virSecurityDACSetGroup(virSecurityManagerPtr mgr, - gid_t group); +void virSecurityDACSetUIDGID(virSecurityManagerPtr mgr, + uid_t user, gid_t group, + gid_t *groups, int ngroups); void virSecurityDACSetDynamicOwnership(virSecurityManagerPtr mgr, bool dynamic); diff --git a/src/security/security_manager.c b/src/security/security_manager.c index 6946637..92ff264 100644 --- a/src/security/security_manager.c +++ b/src/security/security_manager.c @@ -136,6 +136,9 @@ virSecurityManagerPtr virSecurityManagerNewDAC(const char *virtDriver, bool requireConfined, bool dynamicOwnership) { + gid_t *groups; + int ngroups; + virSecurityManagerPtr mgr = virSecurityManagerNewDriver(&virSecurityDriverDAC, virtDriver, @@ -146,8 +149,12 @@ virSecurityManagerPtr virSecurityManagerNewDAC(const char *virtDriver, if (!mgr) return NULL; - virSecurityDACSetUser(mgr, user); - virSecurityDACSetGroup(mgr, group); + if ((ngroups = virGetGroupList(user, group, &groups)) < 0) { + virObjectUnref(mgr); + return NULL; + } + + virSecurityDACSetUIDGID(mgr, user, group, groups, ngroups); virSecurityDACSetDynamicOwnership(mgr, dynamicOwnership); return mgr; -- 1.8.1.4

2 2

[libvirt] [PATCH v3 0/7] Support CHAP authentication for iscsi pool
by John Ferlan 15 Jul '13

15 Jul '13

Based on review comments of v2 posting (see 2/7 for specifics): https://www.redhat.com/archives/libvir-list/2013-July/msg00554.html I rewrote the storage_conf.c chap parsing code. I think the first 6 patches could be squashed into 1 for a final submit, but figured I'd post in steps to make the reviews a bit easier on the eyes. Difference to v2: * Remove the 'login' and 'password' from the AuthChap structure * Reordered some of the steps taken in v2, code is essentially the same, but the process to get there a bit different. * The result is the 'ceph' and 'chap' <auth types are for all intents the same except for a few letters ('eph' and 'hap') * Updated Osier's change to storage_backend_iscsi.c in order to remove the 'login'/'passwd' options. Ran 'make' & 'make check' each step along the way. Also tested with valgrind with no new errors. John Ferlan (6): storage_conf: Adjust virStoragePoolAuthType enum storage_conf: Introduce virStoragePoolAuthSecretPtr storage_conf: Move auth processing into virStoragePoolDefParseAuth storage_pool: Rework chap XML to mimic ceph storage_conf: Move username processing into common function storage_conf: Merge AuthChap and AuthCephx into AuthSecret Osier Yang (1): storage: Support "chap" authentication for iscsi pool docs/formatsecret.html.in | 10 +- docs/formatstorage.html.in | 25 +++- docs/schemas/storagepool.rng | 20 +-- src/conf/storage_conf.c | 150 +++++++++++---------- src/conf/storage_conf.h | 21 ++- src/storage/storage_backend_iscsi.c | 107 ++++++++++++++- src/storage/storage_backend_rbd.c | 13 +- tests/storagepoolxml2xmlin/pool-iscsi-auth.xml | 4 +- .../pool-iscsi-vendor-product.xml | 4 +- tests/storagepoolxml2xmlout/pool-iscsi-auth.xml | 4 +- .../pool-iscsi-vendor-product.xml | 4 +- tests/storagepoolxml2xmlout/pool-rbd.xml | 2 +- 12 files changed, 254 insertions(+), 110 deletions(-) -- 1.8.1.4

3 18

[libvirt] Biweekly upstream qemu-kvm test report - July 15th, 2013
by chandrashekar shastri 15 Jul '13

15 Jul '13

Hi, Please find the status of the upstream testing: Kernel : 3.10.0-rc5+ Qemu : 1.5.50 Libvirt : 1.0.6 Bugs filed in this week : 5 Qemu Bugs in Launchpad : ----------------------------------- Bug# 1199416 - Hot-add qcow2 [virtio-scsi] devices doesn't work in SlLES-11-SP2guest https://bugs.launchpad.net/ubuntu/+bug/1199416 Libvirt Bugs Redhat Bugzilla: --------------------------------------- Bug 982224 - Attaching of the Virtio-scsi [qcow2] drives fails with "error: internal error No more available PCI addresses" https://bugzilla.redhat.com/show_bug.cgi?id=982224 Bug 982455 - RHEL Guest fails to boot after attaching 200+ scsi devices [virtio-scsi qcow2] https://bugzilla.redhat.com/show_bug.cgi?id=982455 Bug 980954 - Virtio-scsi drives in Windows7 shows yellow bang in device manager though virtio scsi pass through driver is installed https://bugzilla.redhat.com/show_bug.cgi?id=980954 Bug 982630 - Documentation : virsh attach-disk --help shouldbe updated with proper examples for --type and --driver https://bugzilla.redhat.com/show_bug.cgi?id=982630 Features tested in this week: 1. Virtio-scsi Features that will be taken up in the next cycle: 1. Chardev hotplug 2. VirtFS 3. Virt Guest Suspend Hibernate Thanks, Shastri

2 1

[libvirt] [PATCH] qemu: add macvlan delete to qemuDomainAttachNetDevice cleanup
by Viktor Mihajlovski 15 Jul '13

15 Jul '13

From: Matthew Rosato <mjrosato(a)linux.vnet.ibm.com> If an error occurs during qemuDomainAttachNetDevice after the macvtap was created in qemuPhysIfaceConnect, the macvtap device gets left behind. This patch adds code to the cleanup routine to delete the macvtap. Signed-off-by: Matthew Rosato <mjrosato(a)linux.vnet.ibm.com> Reviewed-by: Viktor Mihajlovski <mihajlov(a)linux.vnet.ibm.com> --- src/qemu/qemu_hotplug.c | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c index 46875ad..c6045a0 100644 --- a/src/qemu/qemu_hotplug.c +++ b/src/qemu/qemu_hotplug.c @@ -965,6 +965,16 @@ cleanup: if (iface_connected) { virDomainConfNWFilterTeardown(net); + if (virDomainNetGetActualType(net) == VIR_DOMAIN_NET_TYPE_DIRECT) { + ignore_value(virNetDevMacVLanDeleteWithVPortProfile( + net->ifname, &net->mac, + virDomainNetGetActualDirectDev(net), + virDomainNetGetActualDirectMode(net), + virDomainNetGetActualVirtPortProfile(net), + cfg->stateDir)); + VIR_FREE(net->ifname); + } + vport = virDomainNetGetActualVirtPortProfile(net); if (vport && vport->virtPortType == VIR_NETDEV_VPORT_PROFILE_OPENVSWITCH) ignore_value(virNetDevOpenvswitchRemovePort( -- 1.7.9.5

3 4

[libvirt] [PATCHv2 0/2] pci: make virPCIDeviceReset more autonomous
by Laine Stump 15 Jul '13

15 Jul '13

This replaces the earlier single patch by the same name (which I somehow managed to send without testing it :-() This time it's split into two patches because some of the static functions in virpci.c needed re-ordering for successful compilation, so I made that movement a separate patch. Laine Stump (2): pci: reorder static functions pci: make virPCIDeviceReset more autonomous src/qemu/qemu_hostdev.c | 6 +- src/qemu/qemu_hotplug.c | 5 +- src/util/virpci.c | 204 +++++++++++++++++++++++++++--------------------- 3 files changed, 118 insertions(+), 97 deletions(-) -- 1.7.11.7

2 6

[libvirt] [PATCH v3] Make logical pools independent on target path
by Martin Kletzander 15 Jul '13

15 Jul '13

When using logical pools, we had to trust the target->path provided. This parameter, however, can be completely ommited and we can get the correct path using 'lvdisplay' command. In order not to omit the target.path completely, we rather default it to '/dev/<source.name>' which is used to check the pool anyway. Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=952973 Signed-off-by: Martin Kletzander <mkletzan(a)redhat.com> --- Notes: v3: - just rebase v2: - don't drop target.path, but fix it instead to '/dev/<source.name>' Thanks to the change from v1, we can now safely drop all the hunks from logical backend and even the dependency on lvdisplay command. There might be some systems where the path is different and the part of this patch using lvdisplay command would make most of them work. However, checkPool() still depends on '/dev/<source.name>' and I haven't found any other way how to fix that, so feel free to ACK just the {conf,docs,tests}/ part in case you think that we shouldn't try to support anything else than '/dev/<source.name>'. configure.ac | 4 + docs/schemas/storagepool.rng | 13 +++- src/conf/storage_conf.c | 19 +++-- src/storage/storage_backend_logical.c | 88 ++++++++++++++-------- src/storage/storage_driver.c | 2 +- tests/storagepoolxml2xmlin/pool-logical-create.xml | 2 +- tests/storagepoolxml2xmlin/pool-logical-nopath.xml | 18 +++++ .../storagepoolxml2xmlout/pool-logical-nopath.xml | 19 +++++ tests/storagepoolxml2xmltest.c | 1 + 9 files changed, 125 insertions(+), 41 deletions(-) create mode 100644 tests/storagepoolxml2xmlin/pool-logical-nopath.xml create mode 100644 tests/storagepoolxml2xmlout/pool-logical-nopath.xml diff --git a/configure.ac b/configure.ac index b5af0d3..967e70a 100644 --- a/configure.ac +++ b/configure.ac @@ -1601,6 +1601,7 @@ if test "$with_storage_lvm" = "yes" || test "$with_storage_lvm" = "check"; then AC_PATH_PROG([PVS], [pvs], [], [$PATH:/sbin:/usr/sbin]) AC_PATH_PROG([VGS], [vgs], [], [$PATH:/sbin:/usr/sbin]) AC_PATH_PROG([LVS], [lvs], [], [$PATH:/sbin:/usr/sbin]) + AC_PATH_PROG([LVDISPLAY], [lvdisplay], [], [$PATH:/sbin:/usr/sbin]) if test "$with_storage_lvm" = "yes" ; then if test -z "$PVCREATE" ; then AC_MSG_ERROR([We need pvcreate for LVM storage driver]) ; fi @@ -1615,6 +1616,7 @@ if test "$with_storage_lvm" = "yes" || test "$with_storage_lvm" = "check"; then if test -z "$PVS" ; then AC_MSG_ERROR([We need pvs for LVM storage driver]) ; fi if test -z "$VGS" ; then AC_MSG_ERROR([We need vgs for LVM storage driver]) ; fi if test -z "$LVS" ; then AC_MSG_ERROR([We need lvs for LVM storage driver]) ; fi + if test -z "$LVDISPLAY" ; then AC_MSG_ERROR([We need lvdisplay for LVM storage driver]) ; fi else if test -z "$PVCREATE" ; then with_storage_lvm=no ; fi if test -z "$VGCREATE" ; then with_storage_lvm=no ; fi @@ -1628,6 +1630,7 @@ if test "$with_storage_lvm" = "yes" || test "$with_storage_lvm" = "check"; then if test -z "$PVS" ; then with_storage_lvm=no ; fi if test -z "$VGS" ; then with_storage_lvm=no ; fi if test -z "$LVS" ; then with_storage_lvm=no ; fi + if test -z "$LVDISPLAY" ; then with_storage_lvm=no ; fi if test "$with_storage_lvm" = "check" ; then with_storage_lvm=yes ; fi fi @@ -1646,6 +1649,7 @@ if test "$with_storage_lvm" = "yes" || test "$with_storage_lvm" = "check"; then AC_DEFINE_UNQUOTED([PVS],["$PVS"],[Location of pvs program]) AC_DEFINE_UNQUOTED([VGS],["$VGS"],[Location of vgs program]) AC_DEFINE_UNQUOTED([LVS],["$LVS"],[Location of lvs program]) + AC_DEFINE_UNQUOTED([LVDISPLAY],["$LVDISPLAY"],[Location of lvdisplay program]) fi fi AM_CONDITIONAL([WITH_STORAGE_LVM], [test "$with_storage_lvm" = "yes"]) diff --git a/docs/schemas/storagepool.rng b/docs/schemas/storagepool.rng index 3c2158a..1b3f4bc 100644 --- a/docs/schemas/storagepool.rng +++ b/docs/schemas/storagepool.rng @@ -62,7 +62,7 @@ <ref name='commonmetadata'/> <ref name='sizing'/> <ref name='sourcelogical'/> - <ref name='target'/> + <ref name='targetlogical'/> </define> <define name='pooldisk'> @@ -207,6 +207,17 @@ </element> </define> + <define name='targetlogical'> + <element name='target'> + <optional> + <element name='path'> + <ref name='absFilePath'/> + </element> + </optional> + <ref name='permissions'/> + </element> + </define> + <define name='sourceinfohost'> <oneOrMore> <element name='host'> diff --git a/src/conf/storage_conf.c b/src/conf/storage_conf.c index 524a4d6..8fe9e3a 100644 --- a/src/conf/storage_conf.c +++ b/src/conf/storage_conf.c @@ -1,7 +1,7 @@ /* * storage_conf.c: config handling for storage driver * - * Copyright (C) 2006-2012 Red Hat, Inc. + * Copyright (C) 2006-2013 Red Hat, Inc. * Copyright (C) 2006-2008 Daniel P. Berrange * * This library is free software; you can redistribute it and/or @@ -953,15 +953,22 @@ virStoragePoolDefParseXML(xmlXPathContextPtr ctxt) /* When we are working with a virtual disk we can skip the target * path and permissions */ if (!(options->flags & VIR_STORAGE_POOL_SOURCE_NETWORK)) { - if (!(target_path = virXPathString("string(./target/path)", ctxt))) { - virReportError(VIR_ERR_XML_ERROR, "%s", - _("missing storage pool target path")); - goto error; + if (ret->type == VIR_STORAGE_POOL_LOGICAL) { + if (virAsprintf(&target_path, "/dev/%s", ret->source.name) < 0) { + virReportOOMError(); + goto error; + } + } else { + target_path = virXPathString("string(./target/path)", ctxt); + if (!target_path) { + virReportError(VIR_ERR_XML_ERROR, "%s", + _("missing storage pool target path")); + goto error; + } } ret->target.path = virFileSanitizePath(target_path); if (!ret->target.path) goto error; - if (virStorageDefParsePerms(ctxt, &ret->target.perms, "./target/permissions", DEFAULT_POOL_PERM_MODE) < 0) diff --git a/src/storage/storage_backend_logical.c b/src/storage/storage_backend_logical.c index 416a042..310286d 100644 --- a/src/storage/storage_backend_logical.c +++ b/src/storage/storage_backend_logical.c @@ -45,6 +45,37 @@ #define PV_BLANK_SECTOR_SIZE 512 +static char * +virStorageBackendGetVolPath(const char *poolname, const char *volname) +{ + char *start = NULL; + char *lvpath = NULL; + char *output = NULL; + + virCommandPtr cmd = virCommandNewArgList(LVDISPLAY, + "--columns", + "--options", "lv_path", + "--noheadings", + "--unbuffered", + NULL); + + virCommandAddArgFormat(cmd, "%s/%s", poolname, volname); + virCommandSetOutputBuffer(cmd, &output); + + if (virCommandRun(cmd, NULL) < 0 || + !(start = strchr(output, '/'))) { + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("Cannot get LV path")); + goto cleanup; + } + + ignore_value(VIR_STRDUP(lvpath, start)); + + cleanup: + VIR_FREE(output); + virCommandFree(cmd); + return lvpath; +} + static int virStorageBackendLogicalSetActive(virStoragePoolObjPtr pool, int on) @@ -113,8 +144,7 @@ virStorageBackendLogicalMakeVol(virStoragePoolObjPtr pool, } if (vol->target.path == NULL) { - if (virAsprintf(&vol->target.path, "%s/%s", - pool->def->target.path, vol->name) < 0) + if (VIR_STRDUP(vol->target.path, groups[10]) < 0) goto cleanup; } @@ -125,9 +155,10 @@ virStorageBackendLogicalMakeVol(virStoragePoolObjPtr pool, * (lvs outputs "[$lvname_vorigin] for field "origin" if the * lv is created with "--virtualsize"). */ - if (groups[1] && !STREQ(groups[1], "") && (groups[1][0] != '[')) { - if (virAsprintf(&vol->backingStore.path, "%s/%s", - pool->def->target.path, groups[1]) < 0) + if (groups[1] && STRNEQ(groups[1], "") && (groups[1][0] != '[')) { + vol->backingStore.path = virStorageBackendGetVolPath(pool->def->source.name, + groups[1]); + if (!vol->backingStore.path) goto cleanup; vol->backingStore.format = VIR_STORAGE_POOL_LOGICAL_LVM2; @@ -263,21 +294,21 @@ virStorageBackendLogicalFindLVs(virStoragePoolObjPtr pool, virStorageVolDefPtr vol) { /* - * # lvs --separator , --noheadings --units b --unbuffered --nosuffix --options \ - * "lv_name,origin,uuid,devices,seg_size,vg_extent_size,size,lv_attr" VGNAME + * # lvs --separator '#' --noheadings --units b --unbuffered --nosuffix --options \ + * "lv_name,origin,uuid,devices,seg_size,vg_extent_size,size,lv_attr,lv_path" VGNAME * - * RootLV,,06UgP5-2rhb-w3Bo-3mdR-WeoL-pytO-SAa2ky,/dev/hda2(0),5234491392,33554432,5234491392,-wi-ao - * SwapLV,,oHviCK-8Ik0-paqS-V20c-nkhY-Bm1e-zgzU0M,/dev/hda2(156),1040187392,33554432,1040187392,-wi-ao - * Test2,,3pg3he-mQsA-5Sui-h0i6-HNmc-Cz7W-QSndcR,/dev/hda2(219),1073741824,33554432,1073741824,owi-a- - * Test3,,UB5hFw-kmlm-LSoX-EI1t-ioVd-h7GL-M0W8Ht,/dev/hda2(251),2181038080,33554432,2181038080,-wi-a- - * Test3,Test2,UB5hFw-kmlm-LSoX-EI1t-ioVd-h7GL-M0W8Ht,/dev/hda2(187),1040187392,33554432,1040187392,swi-a- + * RootLV##06UgP5-2rhb-w3Bo-3mdR-WeoL-pytO-SAa2ky#/dev/hda2(0)#5234491392#33554432#5234491392#-wi-ao#/dev/VGNAME/RootLV + * SwapLV##oHviCK-8Ik0-paqS-V20c-nkhY-Bm1e-zgzU0M#/dev/hda2(156)#1040187392#33554432#1040187392#-wi-ao#/dev/VGNAME/SwapLV + * Test2##3pg3he-mQsA-5Sui-h0i6-HNmc-Cz7W-QSndcR#/dev/hda2(219)#1073741824#33554432#1073741824#owi-a-#/dev/VGNAME/Test2 + * Test3##UB5hFw-kmlm-LSoX-EI1t-ioVd-h7GL-M0W8Ht#/dev/hda2(251)#2181038080#33554432#2181038080#-wi-a-#/dev/VGNAME/Test3 + * Test3#Test2#UB5hFw-kmlm-LSoX-EI1t-ioVd-h7GL-M0W8Ht#/dev/hda2(187)#1040187392#33554432#1040187392#swi-a-#/dev/VGNAME/Test3 * * Pull out name, origin, & uuid, device, device extent start #, * segment size, extent size, size, attrs * * NB can be multiple rows per volume if they have many extents * - * NB lvs from some distros (e.g. SLES10 SP2) outputs trailing "," on each line + * NB lvs from some distros (e.g. SLES10 SP2) outputs trailing separator on each line * * NB Encrypted logical volumes can print ':' in their name, so it is * not a suitable separator (rhbz 470693). @@ -285,7 +316,7 @@ virStorageBackendLogicalFindLVs(virStoragePoolObjPtr pool, * striped, so "," is not a suitable separator either (rhbz 727474). */ const char *regexes[] = { - "^\\s*(\\S+)#(\\S*)#(\\S+)#(\\S+)#(\\S+)#([0-9]+)#(\\S+)#([0-9]+)#([0-9]+)#(\\S+)#?\\s*$" + "^\\s*(\\S+)#(\\S*)#(\\S+)#(\\S+)#(\\S+)#([0-9]+)#(\\S+)#([0-9]+)#([0-9]+)#(\\S+)#(\\S+)#?\\s*$" }; int vars[] = { 10 @@ -300,7 +331,7 @@ virStorageBackendLogicalFindLVs(virStoragePoolObjPtr pool, "--unbuffered", "--nosuffix", "--options", - "lv_name,origin,uuid,devices,segtype,stripes,seg_size,vg_extent_size,size,lv_attr", + "lv_name,origin,uuid,devices,segtype,stripes,seg_size,vg_extent_size,size,lv_attr,lv_path", pool->def->source.name, NULL); if (virStorageBackendRunProgRegex(pool, @@ -454,17 +485,7 @@ virStorageBackendLogicalCheckPool(virConnectPtr conn ATTRIBUTE_UNUSED, virStoragePoolObjPtr pool, bool *isActive) { - char *path; - - *isActive = false; - if (virAsprintf(&path, "/dev/%s", pool->def->source.name) < 0) - return -1; - - if (access(path, F_OK) == 0) - *isActive = true; - - VIR_FREE(path); - + *isActive = (access(pool->def->target.path, F_OK) == 0); return 0; } @@ -684,6 +705,7 @@ virStorageBackendLogicalCreateVol(virConnectPtr conn, int fd = -1; virCommandPtr cmd = NULL; virErrorPtr err; + bool created = false; if (vol->target.encryption != NULL) { virReportError(VIR_ERR_CONFIG_UNSUPPORTED, @@ -699,11 +721,6 @@ virStorageBackendLogicalCreateVol(virConnectPtr conn, VIR_FREE(vol->target.path); } - if (virAsprintf(&vol->target.path, "%s/%s", - pool->def->target.path, - vol->name) == -1) - return -1; - cmd = virCommandNewArgList(LVCREATE, "--name", vol->name, NULL); @@ -722,9 +739,15 @@ virStorageBackendLogicalCreateVol(virConnectPtr conn, if (virCommandRun(cmd, NULL) < 0) goto error; + created = true; virCommandFree(cmd); cmd = NULL; + vol->target.path = virStorageBackendGetVolPath(pool->def->source.name, + vol->name); + if (!vol->target.path) + goto error; + if ((fd = virStorageBackendVolOpen(vol->target.path)) < 0) goto error; @@ -764,7 +787,8 @@ virStorageBackendLogicalCreateVol(virConnectPtr conn, error: err = virSaveLastError(); VIR_FORCE_CLOSE(fd); - virStorageBackendLogicalDeleteVol(conn, pool, vol, 0); + if (created) + virStorageBackendLogicalDeleteVol(conn, pool, vol, 0); virCommandFree(cmd); virSetError(err); virFreeError(err); diff --git a/src/storage/storage_driver.c b/src/storage/storage_driver.c index a8eb731..43bf6de 100644 --- a/src/storage/storage_driver.c +++ b/src/storage/storage_driver.c @@ -1,7 +1,7 @@ /* * storage_driver.c: core driver for storage APIs * - * Copyright (C) 2006-2012 Red Hat, Inc. + * Copyright (C) 2006-2013 Red Hat, Inc. * Copyright (C) 2006-2008 Daniel P. Berrange * * This library is free software; you can redistribute it and/or diff --git a/tests/storagepoolxml2xmlin/pool-logical-create.xml b/tests/storagepoolxml2xmlin/pool-logical-create.xml index 4c67089..fd551e0 100644 --- a/tests/storagepoolxml2xmlin/pool-logical-create.xml +++ b/tests/storagepoolxml2xmlin/pool-logical-create.xml @@ -10,7 +10,7 @@ <device path="/dev/sdb3"/> </source> <target> - <path>/dev/HostVG</path> + <path>/invalid/nonexistent/path</path> <permissions> <mode>0700</mode> <owner>0</owner> diff --git a/tests/storagepoolxml2xmlin/pool-logical-nopath.xml b/tests/storagepoolxml2xmlin/pool-logical-nopath.xml new file mode 100644 index 0000000..307e2ab --- /dev/null +++ b/tests/storagepoolxml2xmlin/pool-logical-nopath.xml @@ -0,0 +1,18 @@ +<pool type='logical'> + <name>HostVG</name> + <uuid>1c13165a-d0f4-3aee-b447-30fb38789091</uuid> + <capacity>99891544064</capacity> + <allocation>99220455424</allocation> + <available>671088640</available> + <source> + <name>HostVG</name> + <format type='lvm2'/> + </source> + <target> + <permissions> + <mode>0700</mode> + <owner>0</owner> + <group>0</group> + </permissions> + </target> +</pool> diff --git a/tests/storagepoolxml2xmlout/pool-logical-nopath.xml b/tests/storagepoolxml2xmlout/pool-logical-nopath.xml new file mode 100644 index 0000000..07860ef --- /dev/null +++ b/tests/storagepoolxml2xmlout/pool-logical-nopath.xml @@ -0,0 +1,19 @@ +<pool type='logical'> + <name>HostVG</name> + <uuid>1c13165a-d0f4-3aee-b447-30fb38789091</uuid> + <capacity unit='bytes'>0</capacity> + <allocation unit='bytes'>0</allocation> + <available unit='bytes'>0</available> + <source> + <name>HostVG</name> + <format type='lvm2'/> + </source> + <target> + <path>/dev/HostVG</path> + <permissions> + <mode>0700</mode> + <owner>0</owner> + <group>0</group> + </permissions> + </target> +</pool> diff --git a/tests/storagepoolxml2xmltest.c b/tests/storagepoolxml2xmltest.c index 53a7f83..d59cff9 100644 --- a/tests/storagepoolxml2xmltest.c +++ b/tests/storagepoolxml2xmltest.c @@ -87,6 +87,7 @@ mymain(void) DO_TEST("pool-dir"); DO_TEST("pool-fs"); DO_TEST("pool-logical"); + DO_TEST("pool-logical-nopath"); DO_TEST("pool-logical-create"); DO_TEST("pool-disk"); DO_TEST("pool-iscsi"); -- 1.8.3.2

2 3