- Devel - libvirt List Archives

[libvirt] [PATCH v2] qemu: Prevent crash of libvirtd without guest agent configuration
by Alex Jia 16 Jul '13

16 Jul '13

If users haven't configured guest agent then qemuAgentCommand() will dereference a NULL 'mon' pointer, which causes crash of libvirtd. With the patch, when the qemu-ga service isn't running in the guest, a expected error "error: Guest agent is not responding: Guest agent not available for now" will be raised, and the error "error: argument unsupported: QEMU guest agent is not configured" is raised when the guest hasn't configured guest agent. GDB backtrace: (gdb) bt #0 virNetServerFatalSignal (sig=11, siginfo=<value optimized out>, context=<value optimized out>) at rpc/virnetserver.c:326 #1 <signal handler called> #2 qemuAgentCommand (mon=0x0, cmd=0x7f39300017b0, reply=0x7f394b090910, seconds=-2) at qemu/qemu_agent.c:975 #3 0x00007f39429507f6 in qemuAgentGetVCPUs (mon=0x0, info=0x7f394b0909b8) at qemu/qemu_agent.c:1475 #4 0x00007f39429d9857 in qemuDomainGetVcpusFlags (dom=<value optimized out>, flags=9) at qemu/qemu_driver.c:4849 #5 0x00007f3957dffd8d in virDomainGetVcpusFlags (domain=0x7f39300009c0, flags=8) at libvirt.c:9843 How to reproduce? # To start a guest without guest agent configuration # then run the following cmdline # virsh vcpucount foobar --guest error: End of file while reading data: Input/output error error: One or more references were leaked after disconnect from the hypervisor error: Failed to reconnect to the hypervisor RHBZ: https://bugzilla.redhat.com/show_bug.cgi?id=984821 Signed-off-by: Alex Jia <ajia(a)redhat.com> --- src/qemu/qemu_driver.c | 26 ++++++++++++++++++++++++++ 1 files changed, 26 insertions(+), 0 deletions(-) diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 495867a..699388c 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -4122,6 +4122,19 @@ qemuDomainSetVcpusFlags(virDomainPtr dom, unsigned int nvcpus, goto endjob; } + if (priv->agentError) { + virReportError(VIR_ERR_AGENT_UNRESPONSIVE, "%s", + _("QEMU guest agent is not " + "available due to an error")); + goto cleanup; + } + + if (!priv->agent) { + virReportError(VIR_ERR_ARGUMENT_UNSUPPORTED, "%s", + _("QEMU guest agent is not configured")); + goto cleanup; + } + qemuDomainObjEnterAgent(vm); ncpuinfo = qemuAgentGetVCPUs(priv->agent, &cpuinfo); qemuDomainObjExitAgent(vm); @@ -4839,6 +4852,19 @@ qemuDomainGetVcpusFlags(virDomainPtr dom, unsigned int flags) if (qemuDomainObjBeginJob(driver, vm, QEMU_JOB_QUERY) < 0) goto cleanup; + if (priv->agentError) { + virReportError(VIR_ERR_AGENT_UNRESPONSIVE, "%s", + _("QEMU guest agent is not " + "available due to an error")); + goto cleanup; + } + + if (!priv->agent) { + virReportError(VIR_ERR_ARGUMENT_UNSUPPORTED, "%s", + _("QEMU guest agent is not configured")); + goto cleanup; + } + if (!virDomainObjIsActive(vm)) { virReportError(VIR_ERR_OPERATION_INVALID, "%s", _("domain is not running")); -- 1.7.1

2 2

[libvirt] [PATCH v4 0/3] Support CHAP authentication for iscsi pool
by John Ferlan 16 Jul '13

16 Jul '13

https://bugzilla.redhat.com/show_bug.cgi?id=957294 Based on reviews of the the v2/v3 changes to the storage backend drivers regarding the call to perform the authentication, see (and followups): https://www.redhat.com/archives/libvir-list/2013-July/msg00910.html I moved the authentication of the pool back into the startPool callback instread of the findPoolSources() as was done for the original patches, see: https://www.redhat.com/archives/libvir-list/2013-May/msg01887.html https://www.redhat.com/archives/libvir-list/2013-May/msg01886.html Although the plain text option was removed. In order to achieve the goal of getting the secret, the startPool path needed a connection to a driver, so like the nwfilter_driver I chose a qemu connection. Deciding whether to use a privileged connection or not was made based on the privileged value set during storage driver state initialization. Maintaining that state allows for the decision further in the storageDriverAutostart() code to make the connection. Adjusted the existing rbd authentication to take advantage of this as well. >From what I see in just reading the code, this path would have sent a NULL 'conn' to the virStorageBackendRBDOpenRADOSConn() via the call from volStorageBackendRBDRefreshVolInfo() (eg, refreshPool()) and would have failed with a "failed to find the secret"). Whether this was a latent issue or not - I'm not quite sure. This code follows Osier's original change to be sure to call he secret driver 'secretGetValue' directly rather than through the virSecretGetValue() API. Using the same connection paradigm for the chap/iscsi authentication path in order to access the secret driver 'secretGetValue' directly rather than through the virSecretGetValue() API. John Ferlan (3): Add a privileged field to storageDriverState Adjust 'ceph' authentication secret usage for rbd pool. storage: Support "chap" authentication for iscsi pool src/conf/storage_conf.h | 1 + src/storage/storage_backend_iscsi.c | 111 +++++++++++++++++++++++++++++++++++- src/storage/storage_backend_rbd.c | 21 ++++++- src/storage/storage_driver.c | 19 ++++-- 4 files changed, 145 insertions(+), 7 deletions(-) -- 1.8.1.4

2 6

[libvirt] [v3 PATCH] storage: skip async-deleted volume while fs storage is being refreshed
by Guannan Ren 16 Jul '13

16 Jul '13

Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=977706 v1, v2 by Ján Tomko https://www.redhat.com/archives/libvir-list/2013-July/msg00639.html To get volume file fd in the fist place. The purpose of doing so is to ensure the async deletion of volume doesn't make impact on the following operation for the volume till we recheck the existence of volume file later. If we don't get its fd firstly, it is diffcult to differentiate the non-existent file error caused by system or being deleted asynchronous. --- src/storage/storage_backend.c | 49 +++++++++++++++++----------------------- src/storage/storage_backend_fs.c | 6 +++++ 2 files changed, 27 insertions(+), 28 deletions(-) diff --git a/src/storage/storage_backend.c b/src/storage/storage_backend.c index 8d5880e..aa1635a 100644 --- a/src/storage/storage_backend.c +++ b/src/storage/storage_backend.c @@ -1093,29 +1093,13 @@ virStorageBackendVolOpenCheckMode(const char *path, unsigned int flags) int fd, mode = 0; struct stat sb; char *base = last_component(path); - - if (lstat(path, &sb) < 0) { - virReportSystemError(errno, - _("cannot stat file '%s'"), - path); - return -1; - } - - if (S_ISFIFO(sb.st_mode)) { - VIR_WARN("ignoring FIFO '%s'", path); - return -2; - } else if (S_ISSOCK(sb.st_mode)) { - VIR_WARN("ignoring socket '%s'", path); - return -2; - } + int ret = -1; if ((fd = open(path, O_RDONLY|O_NONBLOCK|O_NOCTTY)) < 0) { - if ((errno == ENOENT || errno == ELOOP) && - S_ISLNK(sb.st_mode)) { - VIR_WARN("ignoring dangling symlink '%s'", path); + if (errno == ENOENT) { + VIR_WARN("volume '%s' does not exist", path); return -2; } - virReportSystemError(errno, _("cannot open volume '%s'"), path); @@ -1126,8 +1110,7 @@ virStorageBackendVolOpenCheckMode(const char *path, unsigned int flags) virReportSystemError(errno, _("cannot stat file '%s'"), path); - VIR_FORCE_CLOSE(fd); - return -1; + goto error; } if (S_ISREG(sb.st_mode)) @@ -1141,26 +1124,36 @@ virStorageBackendVolOpenCheckMode(const char *path, unsigned int flags) if (STREQ(base, ".") || STREQ(base, "..")) { - VIR_FORCE_CLOSE(fd); VIR_INFO("Skipping special dir '%s'", base); - return -2; + goto skipfile; } + } else if (S_ISFIFO(sb.st_mode)) { + VIR_WARN("ignoring FIFO '%s'", path); + goto skipfile; + } else if (S_ISSOCK(sb.st_mode)) { + VIR_WARN("ignoring socket '%s'", path); + goto skipfile; } if (!(mode & flags)) { - VIR_FORCE_CLOSE(fd); - VIR_INFO("Skipping volume '%s'", path); - if (mode & VIR_STORAGE_VOL_OPEN_ERROR) { virReportError(VIR_ERR_INTERNAL_ERROR, _("unexpected storage mode for '%s'"), path); - return -1; + goto error; } - return -2; + VIR_INFO("Skipping volume '%s'", path); + goto skipfile; } return fd; + +skipfile: + ret = -2; + +error: + VIR_FORCE_CLOSE(fd); + return ret; } int virStorageBackendVolOpen(const char *path) diff --git a/src/storage/storage_backend_fs.c b/src/storage/storage_backend_fs.c index d305b06..d0276fc 100644 --- a/src/storage/storage_backend_fs.c +++ b/src/storage/storage_backend_fs.c @@ -889,6 +889,12 @@ virStorageBackendFileSystemRefresh(virConnectPtr conn ATTRIBUTE_UNUSED, } } + /* Recheck the existence of volume again */ + if (access(vol->target.path, F_OK) < 0) { + virStorageVolDefFree(vol); + vol = NULL; + continue; + } if (VIR_REALLOC_N(pool->volumes.objs, pool->volumes.count+1) < 0) -- 1.8.3.1

1 0

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

16 Jul '13

When using logical pools, we had to trust the target->path provided. This parameter, however, can be completely ommited and we can use '/dev/<source.name>' safely and populate it to target.path. Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=952973 Signed-off-by: Martin Kletzander <mkletzan(a)redhat.com> --- Notes: v4: - Don't add anything complicated, just rely on /dev/VG_NAME/LV_NAME v3: - just rebase v2: - don't drop pool's target.path, but fix it instead to /dev/VG_NAME docs/schemas/storagepool.rng | 13 ++++++++++++- src/conf/storage_conf.c | 17 ++++++++++++----- src/storage/storage_backend_logical.c | 22 +++------------------- src/storage/storage_driver.c | 2 +- tests/storagepoolxml2xmlin/pool-logical-create.xml | 2 +- tests/storagepoolxml2xmlin/pool-logical-nopath.xml | 19 +++++++++++++++++++ .../storagepoolxml2xmlout/pool-logical-nopath.xml | 22 ++++++++++++++++++++++ tests/storagepoolxml2xmltest.c | 1 + 8 files changed, 71 insertions(+), 27 deletions(-) create mode 100644 tests/storagepoolxml2xmlin/pool-logical-nopath.xml create mode 100644 tests/storagepoolxml2xmlout/pool-logical-nopath.xml 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..a517cbf 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,10 +953,17 @@ 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) { + 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) diff --git a/src/storage/storage_backend_logical.c b/src/storage/storage_backend_logical.c index 416a042..8998a11 100644 --- a/src/storage/storage_backend_logical.c +++ b/src/storage/storage_backend_logical.c @@ -454,17 +454,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; } @@ -794,21 +784,16 @@ virStorageBackendLogicalDeleteVol(virConnectPtr conn ATTRIBUTE_UNUSED, unsigned int flags) { int ret = -1; - char *volpath = NULL; virCommandPtr lvchange_cmd = NULL; virCommandPtr lvremove_cmd = NULL; virCheckFlags(0, -1); - if (virAsprintf(&volpath, "%s/%s", - pool->def->source.name, vol->name) < 0) - goto cleanup; - virFileWaitForDevices(); - lvchange_cmd = virCommandNewArgList(LVCHANGE, "-aln", volpath, NULL); - lvremove_cmd = virCommandNewArgList(LVREMOVE, "-f", volpath, NULL); + lvchange_cmd = virCommandNewArgList(LVCHANGE, "-aln", vol->target.path, NULL); + lvremove_cmd = virCommandNewArgList(LVREMOVE, "-f", vol->target.path, NULL); if (virCommandRun(lvremove_cmd, NULL) < 0) { if (virCommandRun(lvchange_cmd, NULL) < 0) { @@ -821,7 +806,6 @@ virStorageBackendLogicalDeleteVol(virConnectPtr conn ATTRIBUTE_UNUSED, ret = 0; cleanup: - VIR_FREE(volpath); virCommandFree(lvchange_cmd); virCommandFree(lvremove_cmd); return ret; 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..e1bb4db --- /dev/null +++ b/tests/storagepoolxml2xmlin/pool-logical-nopath.xml @@ -0,0 +1,19 @@ +<pool type='logical'> + <name>HostVG</name> + <uuid>1c13165a-d0f4-3aee-b447-30fb38789091</uuid> + <capacity>99891544064</capacity> + <allocation>99220455424</allocation> + <available>671088640</available> + <source> + <device path="/dev/sdb1"/> + <device path="/dev/sdb2"/> + <device path="/dev/sdb3"/> + </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..7413f1c --- /dev/null +++ b/tests/storagepoolxml2xmlout/pool-logical-nopath.xml @@ -0,0 +1,22 @@ +<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> + <device path='/dev/sdb1'/> + <device path='/dev/sdb2'/> + <device path='/dev/sdb3'/> + <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 2

[libvirt] [PATCH v5 0/5] Chardev hotplug
by Michal Privoznik 16 Jul '13

16 Jul '13

I've pushed all ACKed patches from previous rounds. So now just the rest. Michal Privoznik (5): qemuBuildChrDeviceCommandLine: Don't leak devstr domain_conf: Auto fill chardev port qemu: Implement chardev hotplug on config level qemu: Implement chardev hotplug on live level qemuhotplugtest: Introduce test for chardev hotplug src/conf/domain_conf.c | 52 +++++- src/conf/domain_conf.h | 3 + src/libvirt_private.syms | 1 + src/qemu/qemu_command.c | 39 ++++- src/qemu/qemu_driver.c | 42 ++++- src/qemu/qemu_hotplug.c | 175 +++++++++++++++++++ src/qemu/qemu_hotplug.h | 13 ++ tests/qemuhotplugtest.c | 194 +++++++++++++++++---- .../qemuhotplug-console-virtio.xml | 5 + .../qemuxml2argv-console-compat-2.xml | 122 +++++++++++++ 10 files changed, 597 insertions(+), 49 deletions(-) create mode 100644 tests/qemuhotplugtestdata/qemuhotplug-console-virtio.xml create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-console-compat-2.xml -- 1.8.1.5

3 12

[libvirt] [PATCH] qemu: Prevent crash of libvirtd without guest agent configuration
by Alex Jia 16 Jul '13

16 Jul '13

If users haven't configured guest agent then qemuAgentCommand() will dereference a NULL 'mon' pointer, which causes crash of libvirtd. GDB backtrace: (gdb) bt #0 virNetServerFatalSignal (sig=11, siginfo=<value optimized out>, context=<value optimized out>) at rpc/virnetserver.c:326 #1 <signal handler called> #2 qemuAgentCommand (mon=0x0, cmd=0x7f39300017b0, reply=0x7f394b090910, seconds=-2) at qemu/qemu_agent.c:975 #3 0x00007f39429507f6 in qemuAgentGetVCPUs (mon=0x0, info=0x7f394b0909b8) at qemu/qemu_agent.c:1475 #4 0x00007f39429d9857 in qemuDomainGetVcpusFlags (dom=<value optimized out>, flags=9) at qemu/qemu_driver.c:4849 #5 0x00007f3957dffd8d in virDomainGetVcpusFlags (domain=0x7f39300009c0, flags=8) at libvirt.c:9843 How to reproduce? # To start a guest without guest agent configuration # then run the following cmdline # virsh vcpucount foobar --guest error: End of file while reading data: Input/output error error: One or more references were leaked after disconnect from the hypervisor error: Failed to reconnect to the hypervisor RHBZ: https://bugzilla.redhat.com/show_bug.cgi?id=984821 Signed-off-by: Alex Jia <ajia(a)redhat.com> --- src/qemu/qemu_driver.c | 12 ++++++++++++ 1 files changed, 12 insertions(+), 0 deletions(-) diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 495867a..1448aa7 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -4122,6 +4122,12 @@ qemuDomainSetVcpusFlags(virDomainPtr dom, unsigned int nvcpus, goto endjob; } + if (!priv->agent) { + virReportError(VIR_ERR_ARGUMENT_UNSUPPORTED, "%s", + _("QEMU guest agent is not configured")); + goto cleanup; + } + qemuDomainObjEnterAgent(vm); ncpuinfo = qemuAgentGetVCPUs(priv->agent, &cpuinfo); qemuDomainObjExitAgent(vm); @@ -4839,6 +4845,12 @@ qemuDomainGetVcpusFlags(virDomainPtr dom, unsigned int flags) if (qemuDomainObjBeginJob(driver, vm, QEMU_JOB_QUERY) < 0) goto cleanup; + if (!priv->agent) { + virReportError(VIR_ERR_ARGUMENT_UNSUPPORTED, "%s", + _("QEMU guest agent is not configured")); + goto cleanup; + } + if (!virDomainObjIsActive(vm)) { virReportError(VIR_ERR_OPERATION_INVALID, "%s", _("domain is not running")); -- 1.7.1

2 2

[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