Re: [PATCH 1/2] qemu: fix some small issue in qemuProcessAttach

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 




On 12/01/2014 06:27 PM, Martin Kletzander wrote:
On Mon, Dec 01, 2014 at 11:17:54AM +0100, Martin Kletzander wrote:
On Mon, Dec 01, 2014 at 05:54:35PM +0800, Luyao Huang wrote:
There are some small issue in qemuProcessAttach:

1.Fix virSecurityManagerGetProcessLabel always get pid = 0,
move 'vm->pid = pid' before call virSecurityManagerGetProcessLabel.

2.Use virSecurityManagerGenLabel to get image label.

3.Fix always set selinux label for other security driver label.

Signed-off-by: Luyao Huang <lhuang@xxxxxxxxxx>
---
src/qemu/qemu_process.c | 10 +++++++---
1 file changed, 7 insertions(+), 3 deletions(-)


It looks like we were doing everything already, but we just did it
wrong.  Nice catch!  ACK.


Oh, I spoke too soon.  Two minor things are wrong with this patch that
I'll fixup before pushing:

1) The commit message summary is not very descriptive.  When someone
   will go over the git log he/she won't figure out what was the deal
   from "fix some small issue:.

Thanks for pointing out, BTW, maybe qemuprocessattach will failed after this patch without the patch 2/2, because we should give a way to get process uid and gid in virSecurityDACGetProcessLabel otherwise it will always return -1 without a error
settings.

I will edit Patch 2 and give a v2 in these days (otherwise i cannot use qemu-attach : )).

2) see below...

Martin

diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c
index 382d802..ee95adb 100644
--- a/src/qemu/qemu_process.c
+++ b/src/qemu/qemu_process.c
@@ -5255,6 +5255,8 @@ int qemuProcessAttach(virConnectPtr conn ATTRIBUTE_UNUSED,
    if (VIR_STRDUP(priv->pidfile, pidfile) < 0)
        goto error;

+    vm->pid = pid;
+
    VIR_DEBUG("Detect security driver config");
sec_managers = virSecurityManagerGetNested(driver->securityManager);
    if (sec_managers == NULL)
@@ -5272,7 +5274,7 @@ int qemuProcessAttach(virConnectPtr conn ATTRIBUTE_UNUSED,
        seclabeldef->type = VIR_DOMAIN_SECLABEL_STATIC;
        if (VIR_ALLOC(seclabel) < 0)
            goto error;
-        if (virSecurityManagerGetProcessLabel(driver->securityManager,
+        if (virSecurityManagerGetProcessLabel(sec_managers[i],
vm->def, vm->pid, seclabel) < 0)
            goto error;

@@ -5290,6 +5292,10 @@ int qemuProcessAttach(virConnectPtr conn ATTRIBUTE_UNUSED,
        }
    }

+ if (virSecurityManagerGenLabel(driver->securityManager, vm->def) < 0) {
+        goto error;
+    }
+

This won't pass our syntax-check.
Oh that is a accident, i removed the error settings but forgot the {} when i sent this patch : )

    VIR_DEBUG("Creating domain log file");
    if ((logfile = qemuDomainCreateLog(driver, vm, false)) < 0)
        goto error;
@@ -5334,8 +5340,6 @@ int qemuProcessAttach(virConnectPtr conn ATTRIBUTE_UNUSED,

qemuDomainObjTaint(driver, vm, VIR_DOMAIN_TAINT_EXTERNAL_LAUNCH, logfile);

-    vm->pid = pid;
-
    VIR_DEBUG("Waiting for monitor to show up");
if (qemuProcessWaitForMonitor(driver, vm, QEMU_ASYNC_JOB_NONE, priv->qemuCaps, -1) < 0)
        goto error;
--
1.8.3.1

--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list



--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list

--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list




[Index of Archives]     [Virt Tools]     [Libvirt Users]     [Lib OS Info]     [Fedora Users]     [Fedora Desktop]     [Fedora SELinux]     [Big List of Linux Books]     [Yosemite News]     [KDE Users]     [Fedora Tools]