[libvirt] [PATCH] fix a bug of sheepdog storage driver
by harryxiyou＠gmail.com
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
11 years, 9 months
[libvirt] [PATCH 00/15] Permit setting capabilities for uid!=0 processes
by Laine Stump
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
11 years, 9 months
[libvirt] [PATCH] qemu: support vhost-net for generic ethernet devices
by Laine Stump
>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
11 years, 9 months
[libvirt] [PATCH] network_conf.c: Free xmlDoc after use
by Michal Privoznik
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
11 years, 9 months
[libvirt] [PATCH] virsh-snapshot: Fix xpath query to determine snapshot state
by Peter Krempa
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
11 years, 9 months
[libvirt] Why not add snapshot operations for Sheepdog storage driver in Libvirt
by harryxiyou
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
11 years, 9 months
[libvirt] [PATCH] fix some bugs of sheepdog storage driver
by harryxiyou＠gmail.com
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
11 years, 9 months
[libvirt] [PATCH] fix some bugs of sheepdog storage driver
by harryxiyou＠gmail.com
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
11 years, 9 months
Re: [libvirt] [Patch]Fix bugs of Sheepdog storage driver
by harryxiyou
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
11 years, 9 months
