- Devel - libvirt List Archives

Re: [libvirt] Hard link to root-owned file now fails (since Fedora 19)
by Richard W.M. Jones 16 Jul '13

16 Jul '13

On Tue, Jul 16, 2013 at 01:37:33PM -0400, Colin Walters wrote: > But ok, so as you said in the followup, the real root issue here is > libvirt calling chown [on the kernel and initrd]. It's not clear > to me why it does so. It's a good question, and trying to formulate an answer made me question some fundamental assumptions. I'm CC-ing libvir-list to see if anyone has some ideas. ---------------------------------------- Some background: We've got three users involved here. nova libvirtd qemu -- invokes --> -- invokes --> some $UID root qemu.qemu The reason for this dance of three users is security. It provides extra separation in case a rogue filesystem that we're examining exploits the guest kernel and qemu (which is reckoned to be rather easy). We don't want that rogue qemu process to be able to escalate this to an attack on either the host or the process (nova) running libguestfs. SELinux is used too for extra extra security, but you may be amazed that it's not unknown for people to turn SELinux off. ---------------------------------------- Now does libvirtd need to chown kernel & initrd? Ideally (and something that's slowly being worked towards) libvirtd would open all resources on qemu's behalf, and pass opened file descriptors down to qemu. In the case of kernel and initrd libvirtd could do this already by passing /dev/fd/<N> paths to qemu. This, I think, would avoid any need to chown those. For the appliance disk, /dev/fd/<N> also works, but only because we're not hot-plugging this disk. (When you have to hot-plug a disk, you are talking to qemu over the qemu monitor, so the only way to pass fd's over is to use SCM_RIGHTS). Could we just rely on Unix permissions? I think the answer is yes for kernel & initrd (make them world readable), at least in this case. If kernel & initrd had private data, you wouldn't want to do this. The appliance disk is a little more tricky. Rich. -- Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones libguestfs lets you edit virtual machines. Supports shell scripting, bindings from many languages. http://libguestfs.org

1 0

[libvirt] [PATCH 00/12] Remove devices only after DEVICE_DELETED event
by Jiri Denemark 16 Jul '13

16 Jul '13

This series (partially) fixes a longstanding bug in device unplug from a live domain. We considered the QEMU command used for that to be synchronous and removed the unplugged device from domain definition immediately after the command returned success. This is OK for USB devices but other devices actually need guest cooperation to be unplugged and thus they are actually unplugged asynchronously and QEMU tells us about that using DEVICE_DELETED event. This series is not complete, it does not check if any device finished unplug while libvirtd was not running. I'm working on that part but I wanted to get some feedback on this series as soon as possible. Path 3/12 explains how I decided to deal with backward compatibility of the virDomainDetachDeviceFlags API. No libvirt client/app should see any real difference in behavior after this series unless they want to. Jiri Denemark (12): Add VIR_DOMAIN_EVENT_ID_DEVICE_REMOVED event examples: Handle VIR_DOMAIN_EVENT_ID_DEVICE_REMOVED event Clarify virDomainDetachDeviceFlags documentation qemu: Add qemuDomainReleaseDeviceAddress to remove any address qemu: Separate disk device removal into a standalone function qemu: Separate controller removal into a standalone function qemu: Separate net device removal into a standalone function qemu: Separate host device removal into a standalone function Add virDomainDefFindDevice for looking up a device by its alias qemu: Add support for DEVICE_DELETED event qemu: Remove devices only after DEVICE_DELETED event qemu: Emit VIR_DOMAIN_EVENT_ID_DEVICE_REMOVED events daemon/remote.c | 32 + examples/domain-events/events-c/event-test.c | 23 +- examples/domain-events/events-python/event-test.py | 4 + include/libvirt/libvirt.h.in | 18 + python/libvirt-override-virConnect.py | 9 + python/libvirt-override.c | 52 +- src/conf/domain_conf.c | 41 ++ src/conf/domain_conf.h | 4 + src/conf/domain_event.c | 85 ++- src/conf/domain_event.h | 5 + src/libvirt.c | 12 + src/libvirt_private.syms | 3 + src/qemu/qemu_capabilities.c | 2 + src/qemu/qemu_capabilities.h | 1 + src/qemu/qemu_command.c | 35 +- src/qemu/qemu_command.h | 8 +- src/qemu/qemu_domain.h | 2 + src/qemu/qemu_driver.c | 31 +- src/qemu/qemu_hotplug.c | 660 ++++++++++++--------- src/qemu/qemu_hotplug.h | 7 +- src/qemu/qemu_monitor.c | 13 + src/qemu/qemu_monitor.h | 5 + src/qemu/qemu_monitor_json.c | 15 + src/qemu/qemu_process.c | 41 ++ src/remote/remote_driver.c | 32 + src/remote/remote_protocol.x | 13 +- src/remote_protocol-structs | 5 + 27 files changed, 847 insertions(+), 311 deletions(-) -- 1.8.3.2

2 29

[libvirt] [PATCH] Create directory for lease files if it's missing
by Guido Günther 16 Jul '13

16 Jul '13

If we don't autostart a network it's not being created. Debian Bug http://bugs.debian.org/715200 --- src/nwfilter/nwfilter_dhcpsnoop.c | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/src/nwfilter/nwfilter_dhcpsnoop.c b/src/nwfilter/nwfilter_dhcpsnoop.c index 0965f63..3e9f046 100644 --- a/src/nwfilter/nwfilter_dhcpsnoop.c +++ b/src/nwfilter/nwfilter_dhcpsnoop.c @@ -74,8 +74,9 @@ #ifdef HAVE_LIBPCAP -# define LEASEFILE LOCALSTATEDIR "/run/libvirt/network/nwfilter.leases" -# define TMPLEASEFILE LOCALSTATEDIR "/run/libvirt/network/nwfilter.ltmp" +# define LEASEFILE_DIR LOCALSTATEDIR "/run/libvirt/network/" +# define LEASEFILE LEASEFILE_DIR "nwfilter.leases" +# define TMPLEASEFILE LEASEFILE_DIR "nwfilter.ltmp" struct virNWFilterSnoopState { /* lease file */ @@ -1881,6 +1882,11 @@ virNWFilterSnoopLeaseFileRefresh(void) { int tfd; + if (virFileMakePathWithMode(LEASEFILE_DIR, 0700) < 0) { + virReportError(errno, _("mkdir(\"%s\")"), LEASEFILE_DIR); + return; + } + if (unlink(TMPLEASEFILE) < 0 && errno != ENOENT) virReportSystemError(errno, _("unlink(\"%s\")"), TMPLEASEFILE); -- 1.8.3.2

2 3

[libvirt] [PATCH] build: avoid compiler warning on shadowed name
by Eric Blake 16 Jul '13

16 Jul '13

Introduced in commit 24b08219; compilation on RHEL 6.4 complained: qemu/qemu_hotplug.c: In function 'qemuDomainAttachChrDevice': qemu/qemu_hotplug.c:1257: error: declaration of 'remove' shadows a global declaration [-Wshadow] /usr/include/stdio.h:177: error: shadowed declaration is here [-Wshadow] * src/qemu/qemu_hotplug.c (qemuDomainAttachChrDevice): Avoid the name 'remove'. Signed-off-by: Eric Blake <eblake(a)redhat.com> --- Pushing under the build-breaker rule src/qemu/qemu_hotplug.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c index d0198d1..0b7fed6 100644 --- a/src/qemu/qemu_hotplug.c +++ b/src/qemu/qemu_hotplug.c @@ -1254,7 +1254,7 @@ int qemuDomainAttachChrDevice(virQEMUDriverPtr driver, virDomainDefPtr vmdef = vm->def; char *devstr = NULL; char *charAlias = NULL; - bool remove = false; + bool need_remove = false; if (!virQEMUCapsGet(priv->qemuCaps, QEMU_CAPS_DEVICE)) { virReportError(VIR_ERR_OPERATION_INVALID, "%s", @@ -1275,7 +1275,7 @@ int qemuDomainAttachChrDevice(virQEMUDriverPtr driver, if (qemuDomainChrInsert(vmdef, chr) < 0) goto cleanup; - remove = true; + need_remove = true; qemuDomainObjEnterMonitor(driver, vm); if (qemuMonitorAttachCharDev(priv->mon, charAlias, &chr->source) < 0) { @@ -1293,7 +1293,7 @@ int qemuDomainAttachChrDevice(virQEMUDriverPtr driver, ret = 0; cleanup: - if (ret < 0 && remove) + if (ret < 0 && need_remove) qemuDomainChrRemove(vmdef, chr); VIR_FREE(charAlias); VIR_FREE(devstr); -- 1.8.3.1

1 0

[libvirt] [PATCH RESEND 0/7] LXC: enhance user namespace support for adding the disk and host devices for libvirt lxc
by Gao feng 16 Jul '13

16 Jul '13

Libvirt lxc has supported user namespace since commit c34107dfd3a25232255e6d6f559b1306ef99bb3b,but for the disk devices and hostdevs, libvirt lxc still creates these device nodes in container. this will fail when container enables user namespace, since user namespace is disallowed to create device node. In order to reslove this problem, we should create device nodes on host side for container,and change the owner of these nodes to the root user of container. Gao feng (7): LXC: Setup disks for container on host side LXC: controller: change the owner of disk to the root of container LXC: Move virLXCControllerChown to lxc_container.c LXC: Change the owner of live attached disk device LXC: Create host devices for container on host side LXC: Change the owner of host devices to the root of container LXC: Change the owner of live attached host devices src/lxc/lxc_container.c | 382 +++------------------------------------- src/lxc/lxc_container.h | 2 + src/lxc/lxc_controller.c | 450 ++++++++++++++++++++++++++++++++++++++++++++--- src/lxc/lxc_driver.c | 13 ++ 4 files changed, 460 insertions(+), 387 deletions(-) -- 1.8.3.1

3 15

[libvirt] [PATCH] qemu: Fix double free of returned JSON array in qemuAgentGetVCPUs()
by Peter Krempa 16 Jul '13

16 Jul '13

A part of the returned monitor response was freed twice and caused crashes of the daemon when using guest agent cpu count retrieval. # virsh vcpucount dom --guest Introduced in v1.0.6-48-gc6afcb0 --- src/qemu/qemu_agent.c | 1 - 1 file changed, 1 deletion(-) diff --git a/src/qemu/qemu_agent.c b/src/qemu/qemu_agent.c index aca5ff3..72bf211 100644 --- a/src/qemu/qemu_agent.c +++ b/src/qemu/qemu_agent.c @@ -1529,7 +1529,6 @@ qemuAgentGetVCPUs(qemuAgentPtr mon, cleanup: virJSONValueFree(cmd); virJSONValueFree(reply); - virJSONValueFree(data); return ret; } -- 1.8.3.2

2 2

[libvirt] [PATCH] rbd: Do not free the secret if it is not set
by Wido den Hollander 16 Jul '13

16 Jul '13

Not all RBD (Ceph) storage pools have cephx authentication turned on, so "secret" might not be initialized. It could also be that the secret couldn't be located. Only call virSecretFree() if "secret" is initialized earlier. Signed-off-by: Wido den Hollander <wido(a)widodh.nl> --- src/storage/storage_backend_rbd.c | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/src/storage/storage_backend_rbd.c b/src/storage/storage_backend_rbd.c index 493e33b..1f75481 100644 --- a/src/storage/storage_backend_rbd.c +++ b/src/storage/storage_backend_rbd.c @@ -176,7 +176,11 @@ static int virStorageBackendRBDOpenRADOSConn(virStorageBackendRBDStatePtr *ptr, cleanup: VIR_FREE(secret_value); VIR_FREE(rados_key); - virSecretFree(secret); + + if (secret != NULL) { + virSecretFree(secret); + } + virBufferFreeAndReset(&mon_host); VIR_FREE(mon_buff); return ret; -- 1.7.9.5

2 1

[libvirt] [PATCH v3 00/10] Re-enable memballoon driver statistics reporting
by John Ferlan 16 Jul '13

16 Jul '13

This patch set replaces: https://www.redhat.com/archives/libvir-list/2013-July/msg00435.html Changes 01 -> No change 02 & 03 -> Adjust typecaste to switch() and remove "default:" 04 -> Use "unsigned int" as requested 05 -> Add ballooninit, remove get of period before set, remove setting of period during reconnect, other changes per review 06 -> Remove the opaque structure, replace with direct call through "qom-get" to return "guest-stats" and fill in stats[] directly 07 -> 09-> Remove the "flags", "Flags", "FLAGS" (learned something new!) 10 -> Add virsh.pod (hopefully have syntax right). NOTE: The formatdomain.html.in was already updated in 04 to describe the <stats period='#'/> XML syntax. This change describes the virsh command in order to set the period since this is the change where virsh code was adjusted. In 04, all one could do would be virsh edit John Ferlan (10): Add qemuMonitorJSONGetObjectListPaths() method for QMP qom-list command Add qemuMonitorJSONGetObjectProperty() method for QMP qom-get command Add qemuMonitorJSONSetObjectProperty() method for QMP qom-set command Add 'period' for Memballoon statistics gathering capability Determine whether to start balloon memory stats gathering. Add capability to fetch balloon stats Add new public API virDomainSetMemoryStatsPeriodFlags Specify remote protocol for virDomainSetMemoryStatsPeriodFlags Implement the virDomainSetMemoryStatsPeriodFlags for QEMU driver Allow balloon driver collection to be adjusted dynamically docs/formatdomain.html.in | 19 ++ docs/schemas/domaincommon.rng | 7 + include/libvirt/libvirt.h.in | 3 + src/conf/domain_conf.c | 38 +++- src/conf/domain_conf.h | 1 + src/driver.h | 6 + src/libvirt.c | 64 ++++++ src/libvirt_public.syms | 5 + src/qemu/qemu_driver.c | 65 ++++++ src/qemu/qemu_monitor.c | 140 ++++++++++++- src/qemu/qemu_monitor.h | 2 + src/qemu/qemu_monitor_json.c | 460 ++++++++++++++++++++++++++++++++---------- src/qemu/qemu_monitor_json.h | 60 ++++++ src/qemu/qemu_process.c | 14 +- src/remote/remote_driver.c | 1 + src/remote/remote_protocol.x | 15 +- src/remote_protocol-structs | 6 + tests/qemumonitorjsontest.c | 186 +++++++++++++++++ tools/virsh-domain-monitor.c | 66 +++++- tools/virsh.pod | 22 +- 20 files changed, 1060 insertions(+), 120 deletions(-) -- 1.8.1.4

4 38

[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

[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

[libvirt] [PATCH] conf: reject pci-root controllers with non-zero indexes
by Ján Tomko 15 Jul '13

15 Jul '13

https://bugzilla.redhat.com/show_bug.cgi?id=981261 --- src/conf/domain_conf.c | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 5f0366e..602c9a6 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -5668,6 +5668,13 @@ virDomainControllerDefParseXML(xmlNodePtr node, "have an address")); goto error; } + if (def->idx != 0) { + virReportError(VIR_ERR_XML_ERROR, "%s", + _("pci-root controller should have " + "index 0")); + goto error; + } + } default: -- 1.8.1.5

2 2