- Devel - libvirt List Archives

[libvirt] [PATCH] fix a bug of sheepdog storage driver
by harryxiyou＠gmail.com 09 Feb '13

09 Feb '13

Don't try to refresh Sheepdog volume if creating volume fails. Signed-off-by: Harry Wei <harryxiyou(a)gmail.com> --- src/storage/storage_backend_sheepdog.c | 10 +++++++--- 1 files changed, 7 insertions(+), 3 deletions(-) diff --git a/src/storage/storage_backend_sheepdog.c b/src/storage/storage_backend_sheepdog.c index cd18f33..218284d 100644 --- a/src/storage/storage_backend_sheepdog.c +++ b/src/storage/storage_backend_sheepdog.c @@ -156,7 +156,7 @@ virStorageBackendSheepdogCreateVol(virConnectPtr conn ATTRIBUTE_UNUSED, virStorageVolDefPtr vol) { - int ret; + int ret = -1; if (vol->target.encryption != NULL) { virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", @@ -167,10 +167,14 @@ virStorageBackendSheepdogCreateVol(virConnectPtr conn ATTRIBUTE_UNUSED, virCommandPtr cmd = virCommandNewArgList(COLLIE, "vdi", "create", vol->name, NULL); virCommandAddArgFormat(cmd, "%llu", vol->capacity); virStorageBackendSheepdogAddHostArg(cmd, pool); - ret = virCommandRun(cmd, NULL); + if (virCommandRun(cmd, NULL) < 0) + goto cleanup; - virStorageBackendSheepdogRefreshVol(conn, pool, vol); + if (virStorageBackendSheepdogRefreshVol(conn, pool, vol) < 0) + goto cleanup; + ret = 0; +cleanup: virCommandFree(cmd); return ret; } -- 1.7.0.4

4 3

[libvirt] [PATCH 00/15] Permit setting capabilities for uid!=0 processes
by Laine Stump 08 Feb '13

08 Feb '13

There are a bunch of patches here, but each is small and single-purpose to make reviewing easier (and also so that any potential regressions can be easily bisected). The original purpose of the patches was to permit setting CAP_COMPROMISE_KERNEL for non-root qemu processes, since Fedora 18 now requires that in order for generic PCI passthrough to work (the alternative was to always run qemu as root). Although we may not actually want to do that part (if we can convince kernel people to implement CAP_COMPROMISE_KERNEL such that it's only required when *opening* the necessary sysfs file (done by libvirt), rather than for every read/write (done by qemu), then we will not need CAP_COMPROMISE_KERNEL for qemu), but that is just a couple lines in the final patch, and the rest of the series is still useful, as it make dropping/keeping caps truly work for non-root child processes - this has never before been the case. (for example, CAP_SYS_RAWIO is needed for generic scsi passthrough to work, and until now the only way to have that was to run *all* qemus as root). A bit higher level description of what I've done with all the patches: 1) remove the programmable "hook" from virExecWithHook(), since that function was only called from one place, and always with the same hook function. Rename virExecWithHook() to virExec(), and replace the call to that hook with inline code. 2) give virCommand an API to set the intended uid/gid of the command that's going to be run, and use that instead of a "user hook" where appropriate (in the process completely eliminating two hook functions). 3) Also add an API to virCommand to do the final "set the process label" step for selinux/apparmor. 4) Add a new API to the security driver (and use it from qemu) called virSecurityManagerSetChildProcessLabel() which a) is called prior to virCommandRun() rather than from a command "hook" function, b) takes a virCommand, and c) rather than immediately performing the operation (as virSecurityManagerSetProcessLabel() did), merely stores the necessary information in the virCommand so that virExec can perform the operation (setting selinux label, setuid/gid, etc) 5) make a new function combining the setting of uid/gid and maintaining of capabilities, because that is the only way you can set uid!=0 and still maintain capabilities. Use this in virExec() 6) *Finally* set the CAP_COMPROMISE_KERNEL capability unconditionally for all qemu processes. (If we really do have to do this, we may want to consider making it a qemu.conf setting). Laine Stump (15): util: eliminate generic hook from virExecWithHook util: eliminate extra args from virExec util: refactor virCommandHook into virExec and virCommandHandshakeChild util: add virCommandSetUID and virCommandSetGID util: make virSetUIDGID a NOP when uid or gid is -1 qemu: replace exec hook with virCommandSetUID/GID in qemuCaps* qemu: replace exec hook with virCommandSetUID/GID in storage_backend build: define SECDRIVER_LIBS in Makefile.am util: add security label setting to virCommand security: add new virSecurityManagerSetChildProcessLabel API qemu: let virCommand set child process security labels/uid/gid util: drop capabilities immediately after changing uid/gid of child util: virSetUIDGIDWithCaps - change uid while keeping caps util: maintain caps when running command with uid != 0 qemu: set CAP_COMPROMISE_KERNEL so that pci passthrough works src/Makefile.am | 35 ++- src/libvirt_private.syms | 5 + src/qemu/qemu_capabilities.c | 61 ++--- src/qemu/qemu_process.c | 23 +- src/security/security_apparmor.c | 41 +++- src/security/security_dac.c | 24 +- src/security/security_driver.h | 6 +- src/security/security_manager.c | 13 +- src/security/security_manager.h | 6 +- src/security/security_nop.c | 10 +- src/security/security_selinux.c | 34 ++- src/security/security_stack.c | 20 +- src/storage/storage_backend.c | 28 +-- src/util/vircommand.c | 481 ++++++++++++++++++--------------------- src/util/vircommand.h | 9 +- src/util/virutil.c | 113 ++++++++- src/util/virutil.h | 1 + 17 files changed, 543 insertions(+), 367 deletions(-) -- 1.8.1

3 36

[libvirt] [PATCH] qemu: support vhost-net for generic ethernet devices
by Laine Stump 08 Feb '13

08 Feb '13

>From qemu's point of view these are still just tap devices, so there's no reason they shouldn't work with vhost-net; as a matter of fact, Raja Sivaramakrishnan <srajag00(a)yahoo.com> verified on libvir-list that at least the qemu_command.c part of this patch works: https://www.redhat.com/archives/libvir-list/2012-December/msg01314.html (the hotplug case is extrapolation on my part). --- src/qemu/qemu_command.c | 1 + src/qemu/qemu_hotplug.c | 3 +++ 2 files changed, 4 insertions(+) diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index 33f2ecd..6c28123 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -6159,6 +6159,7 @@ qemuBuildCommandLine(virConnectPtr conn, if (actualType == VIR_DOMAIN_NET_TYPE_NETWORK || actualType == VIR_DOMAIN_NET_TYPE_BRIDGE || + actualType == VIR_DOMAIN_NET_TYPE_ETHERNET || actualType == VIR_DOMAIN_NET_TYPE_DIRECT) { /* Attempt to use vhost-net mode for these types of network device */ diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c index 4504f0b..0c28a6a 100644 --- a/src/qemu/qemu_hotplug.c +++ b/src/qemu/qemu_hotplug.c @@ -754,6 +754,9 @@ int qemuDomainAttachNetDevice(virConnectPtr conn, iface_connected = true; if (qemuOpenVhostNet(vm->def, net, priv->qemuCaps, &vhostfd) < 0) goto cleanup; + } else if (actualType == VIR_DOMAIN_NET_TYPE_ETHERNET) { + if (qemuOpenVhostNet(vm->def, net, priv->qemuCaps, &vhostfd) < 0) + goto cleanup; } if (virQEMUCapsGet(priv->qemuCaps, QEMU_CAPS_NET_NAME) || -- 1.7.11.7

2 2

[libvirt] [PATCH] network_conf.c: Free xmlDoc after use
by Michal Privoznik 08 Feb '13

08 Feb '13

The virNetworkObjUpdateParseFile() function was not freeing the xml variable, leaving us with a memory leak. --- src/conf/network_conf.c | 1 + 1 file changed, 1 insertion(+) diff --git a/src/conf/network_conf.c b/src/conf/network_conf.c index c93916d..3604ff7 100644 --- a/src/conf/network_conf.c +++ b/src/conf/network_conf.c @@ -1895,6 +1895,7 @@ virNetworkObjUpdateParseFile(const char *filename, ret = 0; cleanup: + xmlFreeDoc(xml); xmlXPathFreeContext(ctxt); return ret; } -- 1.8.0.2

3 3

[libvirt] [PATCH] virsh-snapshot: Fix xpath query to determine snapshot state
by Peter Krempa 08 Feb '13

08 Feb '13

The query didn't match the external state correctly for offline internal snapshots. --- tools/virsh-snapshot.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tools/virsh-snapshot.c b/tools/virsh-snapshot.c index ba57059..66776e2 100644 --- a/tools/virsh-snapshot.c +++ b/tools/virsh-snapshot.c @@ -983,8 +983,8 @@ cmdSnapshotInfo(vshControl *ctl, const vshCmd *cmd) * external snapshot. */ switch (virXPathBoolean("boolean(/domainsnapshot/memory)", ctxt)) { case 1: - external = virXPathBoolean("boolean(/domainsnapshot/memory/@snapshot=external " - "| /domainsnapshot/disks/disk/@snapshot=external)", + external = virXPathBoolean("boolean(/domainsnapshot/memory[@snapshot='external'] " + "| /domainsnapshot/disks/disk[@snapshot='external'])", ctxt); break; case 0: -- 1.8.1.1

2 2

[libvirt] [PATCH V3 0/2] Add support for QEMU file descriptor sets
by Stefan Berger 08 Feb '13

08 Feb '13

The following patch series adds initial support for QEMU file descriptor sets implementing support for creating the proper command line. Some devices are using the sets now. Regards, Stefan

3 11

[libvirt] Why not add snapshot operations for Sheepdog storage driver in Libvirt
by harryxiyou 08 Feb '13

08 Feb '13

Hi all, Virsh has following operations about snapshot. Snapshot (help keyword 'snapshot') snapshot-create Create a snapshot from XML snapshot-create-as Create a snapshot from a set of args snapshot-current Get or set the current snapshot snapshot-delete Delete a domain snapshot snapshot-dumpxml Dump XML for a domain snapshot snapshot-edit edit XML for a snapshot snapshot-info snapshot information snapshot-list List snapshots for a domain snapshot-parent Get the name of the parent of a snapshot snapshot-revert Revert a domain to a snapshot But i cannot find relevant realization in Sheepdog storage driver in Libvirt. I am sure Sheepdog storage driver, now, cannot support snapshot operations in Libvirt. I wonder whether Sheepdog can realize snapshot operations in Libvirt? Maybe i can finish these jobs. I am not clear about how to let Sheepdog storage driver support snapshot operations. Could anyone give me some suggestions? Thanks in advance. -- Thanks Harry Wei

2 1

[libvirt] [PATCH] fix some bugs of sheepdog storage driver
by harryxiyou＠gmail.com 08 Feb '13

08 Feb '13

Signed-off-by: Harry Wei <harryxiyou(a)gmail.com> --- src/storage/storage_backend_sheepdog.c | 10 ++++++++++ 1 files changed, 10 insertions(+), 0 deletions(-) diff --git a/src/storage/storage_backend_sheepdog.c b/src/storage/storage_backend_sheepdog.c index 218284d..1ce5aa4 100644 --- a/src/storage/storage_backend_sheepdog.c +++ b/src/storage/storage_backend_sheepdog.c @@ -120,6 +120,8 @@ virStorageBackendSheepdogRefreshPool(virConnectPtr conn ATTRIBUTE_UNUSED, virCommandPtr cmd; cmd = virCommandNewArgList(COLLIE, "node", "info", "-r", NULL); + if (cmd == NULL) + return -1; virStorageBackendSheepdogAddHostArg(cmd, pool); virCommandSetOutputBuffer(cmd, &output); ret = virCommandRun(cmd, NULL); @@ -142,6 +144,8 @@ virStorageBackendSheepdogDeleteVol(virConnectPtr conn ATTRIBUTE_UNUSED, virCheckFlags(0, -1); virCommandPtr cmd = virCommandNewArgList(COLLIE, "vdi", "delete", vol->name, NULL); + if (cmd == NULL) + return -1; virStorageBackendSheepdogAddHostArg(cmd, pool); int ret = virCommandRun(cmd, NULL); @@ -165,6 +169,8 @@ virStorageBackendSheepdogCreateVol(virConnectPtr conn ATTRIBUTE_UNUSED, } virCommandPtr cmd = virCommandNewArgList(COLLIE, "vdi", "create", vol->name, NULL); + if (cmd == NULL) + return -1; virCommandAddArgFormat(cmd, "%llu", vol->capacity); virStorageBackendSheepdogAddHostArg(cmd, pool); if (virCommandRun(cmd, NULL) < 0) @@ -251,6 +257,8 @@ virStorageBackendSheepdogRefreshVol(virConnectPtr conn ATTRIBUTE_UNUSED, char *output = NULL; virCommandPtr cmd = virCommandNewArgList(COLLIE, "vdi", "list", vol->name, "-r", NULL); + if (cmd == NULL) + return -1; virStorageBackendSheepdogAddHostArg(cmd, pool); virCommandSetOutputBuffer(cmd, &output); ret = virCommandRun(cmd, NULL); @@ -293,6 +301,8 @@ virStorageBackendSheepdogResizeVol(virConnectPtr conn ATTRIBUTE_UNUSED, virCheckFlags(0, -1); virCommandPtr cmd = virCommandNewArgList(COLLIE, "vdi", "resize", vol->name, NULL); + if (cmd == NULL) + return -1; virCommandAddArgFormat(cmd, "%llu", capacity); virStorageBackendSheepdogAddHostArg(cmd, pool); int ret = virCommandRun(cmd, NULL); -- 1.7.0.4

2 1

[libvirt] [PATCH] fix some bugs of sheepdog storage driver
by harryxiyou＠gmail.com 08 Feb '13

08 Feb '13

Don't try to do other jobs if get cmd NULL. Signed-off-by: Harry Wei <harryxiyou(a)gmail.com> --- src/storage/storage_backend_sheepdog.c | 10 ++++++++++ 1 files changed, 10 insertions(+), 0 deletions(-) diff --git a/src/storage/storage_backend_sheepdog.c b/src/storage/storage_backend_sheepdog.c index 218284d..1ce5aa4 100644 --- a/src/storage/storage_backend_sheepdog.c +++ b/src/storage/storage_backend_sheepdog.c @@ -120,6 +120,8 @@ virStorageBackendSheepdogRefreshPool(virConnectPtr conn ATTRIBUTE_UNUSED, virCommandPtr cmd; cmd = virCommandNewArgList(COLLIE, "node", "info", "-r", NULL); + if (cmd == NULL) + return -1; virStorageBackendSheepdogAddHostArg(cmd, pool); virCommandSetOutputBuffer(cmd, &output); ret = virCommandRun(cmd, NULL); @@ -142,6 +144,8 @@ virStorageBackendSheepdogDeleteVol(virConnectPtr conn ATTRIBUTE_UNUSED, virCheckFlags(0, -1); virCommandPtr cmd = virCommandNewArgList(COLLIE, "vdi", "delete", vol->name, NULL); + if (cmd == NULL) + return -1; virStorageBackendSheepdogAddHostArg(cmd, pool); int ret = virCommandRun(cmd, NULL); @@ -165,6 +169,8 @@ virStorageBackendSheepdogCreateVol(virConnectPtr conn ATTRIBUTE_UNUSED, } virCommandPtr cmd = virCommandNewArgList(COLLIE, "vdi", "create", vol->name, NULL); + if (cmd == NULL) + return -1; virCommandAddArgFormat(cmd, "%llu", vol->capacity); virStorageBackendSheepdogAddHostArg(cmd, pool); if (virCommandRun(cmd, NULL) < 0) @@ -251,6 +257,8 @@ virStorageBackendSheepdogRefreshVol(virConnectPtr conn ATTRIBUTE_UNUSED, char *output = NULL; virCommandPtr cmd = virCommandNewArgList(COLLIE, "vdi", "list", vol->name, "-r", NULL); + if (cmd == NULL) + return -1; virStorageBackendSheepdogAddHostArg(cmd, pool); virCommandSetOutputBuffer(cmd, &output); ret = virCommandRun(cmd, NULL); @@ -293,6 +301,8 @@ virStorageBackendSheepdogResizeVol(virConnectPtr conn ATTRIBUTE_UNUSED, virCheckFlags(0, -1); virCommandPtr cmd = virCommandNewArgList(COLLIE, "vdi", "resize", vol->name, NULL); + if (cmd == NULL) + return -1; virCommandAddArgFormat(cmd, "%llu", capacity); virStorageBackendSheepdogAddHostArg(cmd, pool); int ret = virCommandRun(cmd, NULL); -- 1.7.0.4

3 2

Re: [libvirt] [Patch]Fix bugs of Sheepdog storage driver
by harryxiyou 08 Feb '13

08 Feb '13

On Fri, Feb 8, 2013 at 1:06 PM, Osier Yang <jyang(a)redhat.com> wrote: [...] > Google Groups小组敬上 > > That's bad if one could get one notice like this each time after > reviewing your patch. :-) Not sure how to get rid of this, but > I think it's caused by the permission, so please either change > the google group to public, or don't cc to the list when posting > patch. > Thanks, it's because i add "cloudxy(a)googlegroups.com" ML to the CC'ed list. >> You sent same patch multiple times. It might be caused by the mail >> delay though, I.E, you don't see the one sent out earlier. Perhaps >> you should check you mailbox setting to avoid the small flood. >> Sorry, i am not familiar with 'git send-email --compose' well. So it sends several times. I will fix this problem. [...] >> So, consider to change the commit log to: >> >> Don't try to refresh the volume if creating volume fails. This one is well for me, thanks. >>> >>> Signed-off-by: Harry Wei<harryxiyou(a)gmail.com> >>> --- >>> src/storage/storage_backend_sheepdog.c | 5 ++++- >>> 1 files changed, 4 insertions(+), 1 deletions(-) >>> >>> diff --git a/src/storage/storage_backend_sheepdog.c >>> b/src/storage/storage_backend_sheepdog.c >>> index cd18f33..f987604 100644 >>> --- a/src/storage/storage_backend_sheepdog.c >>> +++ b/src/storage/storage_backend_sheepdog.c >>> @@ -168,9 +168,12 @@ virStorageBackendSheepdogCreateVol(virConnectPtr >>> conn ATTRIBUTE_UNUSED, >>> virCommandAddArgFormat(cmd, "%llu", vol->capacity); >>> virStorageBackendSheepdogAddHostArg(cmd, pool); >>> ret = virCommandRun(cmd, NULL); >>> + if (ret< 0) >>> + goto cleanup; >> >> >> We prefer: >> >> if ((ret = virCommandRun(cmd, NULL)) < 0) >> goto cleanup; >> >> However, you can avoid using the "ret" here by initialize it >> to "-1", with that you can simply do: >> >> if (virCommandRun(cmd, NULL) < 0) >> goto cleanup; >> >>> >>> - virStorageBackendSheepdogRefreshVol(conn, pool, vol); >>> + ret = virStorageBackendSheepdogRefreshVol(conn, pool, vol); >> >> >> And: >> >> if (virStorageBackendSheepdogRefreshVol(conn, pool, vol) < 0) >> goto cleanup; >> >> ret = 0; >> >>> >>> +cleanup: >>> virCommandFree(cmd); >>> return ret; >>> } We cannot do like [...] if (virCommandRun(cmd, NULL) < 0) goto cleanup; [...] if (virStorageBackendSheepdogRefreshVol(conn, pool, vol) < 0) goto cleanup; [...] +cleanup: virCommandFree(cmd); return ret; We must initialize "ret", because if either virCommandRun and virStorageBackendSheepdogRefreshVol has some errors, they will return '-1' to "ret". Then execute "return ret", which who calls virStorageBackendSheepdogCreateVol will know it has happened some errors. So we should modify them like this. [...] if ((ret = virCommandRun(cmd, NULL)) < 0) goto cleanup; [...] if (ret = virStorageBackendSheepdogRefreshVol(conn, pool, vol)) < 0) goto cleanup; cleanup: virCommandFree(cmd); return ret; } Osier Yang, do you agree with me? -- Thanks Harry Wei

2 4